Expose factories #85
Expose factories #85
Conversation
|
Need to get dotty #2435 in, then it should build OK. |
| protected[this] def fromSpecificIterable(coll: Iterable[A]): C | ||
| protected[this] def fromIterable[E](it: Iterable[E]): CC[E] | ||
|
|
||
| def iterableFactory: IterableFactory[CC] |
There was a problem hiding this comment.
This is a tempting design that I considered on several occasions but always backed out because that factory is limited to building unconstrained unary types. It looks promising the way it's used here. I'll have to take a closer look for potential problems I ran into when I tried it.
There was a problem hiding this comment.
Yes, I’d also like to stress that the solution proposed in this PR is very simple but does not support the use cases described in #65 and might require substantial changes to support scala/scala#5233.
This allows to convert any collection to the dynamic type
of any other by writing e.g.
xs.to(ys.iterableFactory)
Also: Define IterableFactory.empty in terms of fromIterable
The commit shows that the fully generic part of TraverseTest works. The rest is more tricky, but i am not convinced we need to support that. I mean, we can always define optionSequence specialized to individual types - there's not a lot to be gained to specify it separately for ordered collections.
Optimize optionSequence (in TraverseTest) when the underlying IterableFactory happens to be an IterableFactoryWithBuilder
julienrf
force-pushed
the
expose-factories
branch
from
8816321 to
c8434cc
Compare
|
I rebased this PR and introduced a subtype of This new type of factory makes it possible to use a builder that targets the right collection type, in |
| val o4 = optionSequence(xs4) | ||
| val o4t: Option[immutable.List[(Int, String)]] = o4 | ||
| val o5: Option[immutable.TreeMap[Int, String]] = o4.map(_.to(immutable.TreeMap)) | ||
| } |
There was a problem hiding this comment.
We see that this PR gives less power than #80:
- if the initial collection is a Sorted set we get back a Set (which not sorted)
- there is no way to get back Map, we have to transform the resulting Iterable[(Int, String)] into a Map, afterwards.
In the case of "traverse" I don't think that’s a big problem. But more generally if we want to be able to define generic operations that work for any type of collection (see #80 for some examples), we need more power.
To provide this power, one solution could be to follow the same pattern as we have here for IterableFactory and IterableFactoryWithBuilder, but for the four variants of factories (SortedIterable, Map, SortedMap).
However, that would not work for sorted collections because we would only get a builder for the initial collection's element type.
In the case of Map collections, I think that would work but would still require users to always write two overloads of their methods (one for each type of constructor kind, ie. we would have to optionSequence methods).
| @@ -26,6 +28,10 @@ trait IterableFactory[+CC[_]] { | |||
|
|
|||
| } | |||
|
|
|||
| trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] { | |||
| def newBuilder[A](): Builder[A, CC[A]] | |||
| } | |||
There was a problem hiding this comment.
As explained in the comment at the bottom of the diff, it would be useful to have more power and also add the following method:
implicit def canBuild[A]: CanBuild[A, CC[A]] = () => newBuilder[A]Where CanBuild is the type defined in #80. If we add this line and if we also add XxxWithBuilder factories for the other kind of factories (SortedIterable, Map and SortedMap), we would be able to implement the tests given in #80.
There was a problem hiding this comment.
I implemented this suggestion in #91.
| } | ||
| val factory = xs.iterableFactory | ||
| factory match { | ||
| case iterableBuilder: IterableFactoryWithBuilder[CC] => |
There was a problem hiding this comment.
The type of IterableFactoryWithBuilder is rather unfortunate. Using a type constructor argument prevents you from writing a pattern match that doesn't generate a warning.
Every collection now has a
iterableFactorymethod that returns theIterableFactoryassociated with the collection type.fromIterablenow is defined only once in terms ofIterableFactory.This lets us revive fully generic forms of
TraverseTest.We could go further and have a subset of Factories that have
newBuildermethods. (Lazy collections should not offer such a method). We could then avoid using an intermediate collection in traverse test for the building, and instead use the builder of the target collection directly.