Make the different kinds of collection companions consistent together

[julienrf]

Also, define an implicit builder factory () => Builder[A, C[A]] that can be used by end-users to retrieve a factory for a given collection type. For instance we can now write Future.sequence (or equivalent) as follows:

def futureSequence[A, C[X] <: Iterable[X]](xs: C[Future[A]])(implicit canBuild: () => Builder[A, C[A]]): Future[C[A]] =
  xs.foldLeft(Future.successful(canBuild.apply())) { (fr, fa) =>
    fr.zipWith(fa)(_ += _)
  }.map(_.result)

Note that I duplicated the collection factories: IterableFactory works only for collection types of kind * -> *, OrderingGuidedFactory works for the same kind of collection types but requires an implicit Ordering, MapFactory works for collection types of kind * -> * -> *, and SortedMapFactory is the same but requires an implicit Ordering for keys. I don’t think this duplication is a problem. Trying to abstract over these difference would not be worth it, in my opinion.

[Ichoran]

Do we gain anything from this beyond just having the type be () => Builder[A, C[A]]?

[julienrf]

I find a little bit nicer to have implicit vals of type CanBuild[…] rather than () => Builder[…]. I find it also a little bit nicer to take parameters of type CanBuild[…] rather than () => Builder[…]. This is a matter of taste…

[Ichoran]

Okay, but in the former case we'd need only
implicit def canBuild[A] = () => newBuilder[A, C[A]] instead of the four lines we have now.

[julienrf]

Alright, I was reluctant to define implicit vals with type Function1 because they can act as implicit conversions, but in the case the type () => _ the corresponding type is Function0 so it’s perfectly fine.

[julienrf]

Actually, I’m thinking that CanBuild also has the following advantages:

• Simpler to use for end-users (e.g. the type of the parameter is CanBuild[A, C[A]] instead of () => Builder[A, C[A]]) ;
• We could add other methods than just apply(): Builder[A, C[A]], like apply(knownSize: Int): Builder[A, C[A]].

[szeiger]

I'm not convinced by this CanBuild abstraction. It adds a bias towards strict builders and yet another concept. We already have companion objects for collections, so why don't we make them first-class? Every companion object could provide itself as an implicit value:

object List extends CollectionCompanion[List] {
  implicit val companion: CollectionCompanion[List] = this
}

This provides convenient access to both, FromIterable and strict builders, as well as other potential features that may be mixed into the companion object.

The limitation to unary type constructors isn't particularly nice. This would be better (if we don't run into any variance problems; I haven't tried it):

object List extends CollectionCompanion[List[Any]] {
  implicit def companion[T]: CollectionCompanion[List[T]] = this
}

companion would not be part of any trait, so collections are free to define it in whatever way is appropriate, e.g.:

object HashMap extends CollectionCompanion[HashMap[Any, Nothing]] {
  implicit def companion[K, V]: CollectionCompanion[HashMap[K, V]] =
    this // may require cast or @uncheckedVariance
}

[szeiger]

Small addition: It doesn't fully cover constrained collections that require more than a type evidence. We can safely cast if variance is an issue but for Sorted, for example, we need the actual Ordered value, not just a proof that it exists.

[szeiger]

Continuing this line of thought, constrained collections cannot implement IterableFactory either, but they could provide an IterableFactory for a specific element type, thus providing the same interface as unconstrained collection types after an implicit lookup. I'll think some more about this and try to apply it to my universal constrained collections PR.

[szeiger]

Something to keep in mind: scala/scala#5233

Rather than "allow me to build whatever I want" the essence of this seems to be "make it work for Maps". It would be nice to get this right by default, which requires moving away from any design that requires a unary type constructor. (The "allow me to build whatever" case can be covered by Views)

[szeiger]

Why was this renamed from IterableFactory to IterableFactories?

[julienrf]

I thought it would make more sense because this trait provides several factory methods. We can just settle on either XxxFactory or XxxFactories…

[odersky]

I also think Factory is the established wording, whether there are several methods or not.

…

On Mon, Mar 6, 2017 at 7:01 PM, Julien Richard-Foy ***@***.*** > wrote: ***@***.**** commented on this pull request. ------------------------------ In src/main/scala/strawman/collection/IterableFactories.scala <#36 (comment)> : > @@ -0,0 +1,26 @@ +package strawman +package collection + +import strawman.collection.mutable.Builder + +import scala.Int + +/** Base trait for instances that can construct a collection from an iterable */ +trait FromIterable[+C[X] <: Iterable[X]] { + def fromIterable[B](it: Iterable[B]): C[B] +} + +/** Base trait for companion objects of collections */ +trait IterableFactories[+C[X] <: Iterable[X]] extends FromIterable[C] { I thought it would make more sense because this trait provides several factory methods. We can just settle on either XxxFactory or XxxFactories… — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwlVu8BEiqWHKlNMiQSpkuTnFy4unqzks5rjEn_gaJpZM4MLOwY> .

-- Martin Odersky EPFL and Lightbend