Pull up the newBuilder method from Buildable to IterableOps. #110
Conversation
Rename newBuilder to newSpecificBuilder. Rename Buildable to StrictOptimizedIterableOps. Add XxxWithBuilder variants to all kinds of factories.
| protected[this] def fromSpecificIterable(coll: Iterable[A]): View[A] = fromIterable(coll) | ||
|
|
||
| protected[this] def newSpecificBuilder(): Builder[A, View[A]] = | ||
| immutable.IndexedSeq.newBuilder().mapResult(_.view) |
There was a problem hiding this comment.
This is how newSpecificBuilder() is implemented in View.
Note that (fortunately) the View factory object does not publicly expose this builder.
| @@ -36,6 +36,9 @@ class LazyList[+A](expr: => LazyList.Evaluated[A]) | |||
|
|
|||
| protected[this] def fromSpecificIterable(coll: collection.Iterable[A]): LazyList[A] = fromIterable(coll) | |||
|
|
|||
| protected[this] def newSpecificBuilder(): Builder[A, LazyList[A]] = | |||
| IndexedSeq.newBuilder().mapResult(_.to(LazyList)) | |||
There was a problem hiding this comment.
And this is the builder for LazyList. Again, it is not publicly exposed.
|
I also just added a commit that removes builders from mutable collections. I don’t think they are useful: the main purpose of builders is to make it possible to build immutable collections in an imperative way. We don’t need that for mutable collections. |
| @@ -47,6 +43,18 @@ object IterableFactory { | |||
|
|
|||
| } | |||
|
|
|||
| trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] { | |||
There was a problem hiding this comment.
What would you think of StrictIterableFactory instead of IterableFactoryWithBuilder?
There was a problem hiding this comment.
I think many of these names are way too long without being fully documenting (which would be the main reason to have long names). We should aim for at most two words describing the type of collection (preferably one), and one word describing the concept except maybe two when it's an unusual case like Ordering. I don't think we should worry about it now, though; let's get the functionality working first, and then worry about compact suggestive names.
|
I have to say I'm not very fond of all of the formulaic boxing and indirection in the factory companions. Isn't there some way using package private to avoid indirection by allowing "delegation" to just be returning the same object with a different type? Regarding allowing strict builders, I like in principle the idea of having them available when needed (though not the easiest thing to use), but I'm having trouble fully grasping the consequences especially for correctness: can you get violations of expected behavior if you don't manually keep both up to date? |
keep both what? |
|
I guess what I mean is: are there both strict and lazy builders for the same collections, or does one just defer to the other? (Even if yes, I guess that's fine; it just should be documented clearly so if you change how something is built, you know to fix both places.) |
|
We will have both strict and lazy builders for the same collections so yes, we should probably document that better. |
|
Any objection to merge this one? I think we can still further discuss whether we want distinct |
And also:
But the main change is that now all
Iterabehave this protected method that makes it possible to create a strict builder for them. This change is supposed to make it simpler to provide default implementations for several methods (such asgroupBy, see #108). It might also simplify #97.