-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Collection strawman2 #819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Collection strawman2 #819
Conversation
I've seen many projects, including Dotty itself where there is code similar to ```scala def average(xs: Iterator[Int]) = xs.sum/xs.size ``` It simply should not compile.
Major change in 48afa75: there is no class view. It's a type alias, and from this type alias there are decorators that add convenient operations for views. |
As of 5281688 views are not part of collection operations. I believe that view method is fundamentally different from foldRight\head\isEmpty and should belong to a different implicit conversion. |
As of b77ab7c, iterator class does not have operations. The motivating example is def average(xs: Iterator[Int]) = xs.sum/xs.size I believe it simply should not compile. User should instead write def average(xs: View[Int]) = xs.sum/xs.size Note that as of 48afa75 this View[T] is an alias to () => Iterable[T], making this code actually do what it is expected to. |
As an argument that Iterator should not have operations that consume it I'll remind about a bug in dotty that was in code for long time and was tricky to find: |
As of 5244c5c, there is a type alias |
/rebuild |
As of 22ca08d LengthType is a value class, that defines common arithmetic operations, and has implicit convertions to Long and from Int. Given provided methods and implicit conversions, those code samples work: val s: List = ...
s(1) // 1 is converted to LengthType through implicit convertion
s.drop(s.length - 1) // arithmetic operations on LengthType
assert(s.length < 100) // comparisons |
I don't think we need to keep this in the PR queue any longer. |
To support discussion in #818.
It's not going to pass tests before #805 is merged.
It has 4 changes that are applied to Martin’s version,
and I believe they could be discussed independently:
dotty-staging@48afa75
Gets rid of view class. View is a simple type alias to () => Iterator[A].
This points the major difference between Iterators and views: iterators are consumed by
any usage, while views are reusable.
dotty-staging@5281688
Makes .view method be a decorator. This is done in order to decouple views from collection higherarhy
and make implementation of custom collections easier. It would also allow to modularise collections library to a higher degree. I believe that the very same should be done to .par.
dotty-staging@b77ab7c
Makes Iterators have only next and hasNext methods. The motivation could be found in this code sample:
Which was taken from old bug in Dotty codebase dotty-staging@03ec379
I believe that this code simply should not compile: both contains and exists have no side effects on
mutable and immutable collections, but it has a silent side-effect on Iterators. It does not follow principle
of least surprise, I would actually say that this is one of most surprising methods for new-comments that
felt the joy of ”you can use the same methods on parallel, mutable and immutable collections”
when they discover that there is ”but be very careful if you use those methods on iterators”.
Ok, so what the change does: it moves all consuming methods from Iterator[A], to a decorator over View[A], that is, to () => Iterator[A]. This follows the patch that makes difference in use-cases of iterator[A] and View[A] more obvious, that I started in previous commit.
dotty-staging@5244c5c
Extracts a LengthType from all the signatures for documentation purposes & makes it be Long for big-data needs.
This change has a problem with compatibility with ScalaJs, as they simulate Longs.