Introduce factories with builders.

Optimize optionSequence (in TraverseTest) when the underlying
IterableFactory happens to be an IterableFactoryWithBuilder

Author: julienrf

src/main/scala/strawman/collection/Factories.scala

@@ -1,7 +1,9 @@
 package strawman
 package collection
 
-import scala.{Any, Int, Ordering, Nothing}
+import strawman.collection.mutable.Builder
+
+import scala.{Any, Int, Nothing, Ordering}
 import scala.annotation.unchecked.uncheckedVariance
 
 /**
@@ -26,6 +28,10 @@ trait IterableFactory[+CC[_]] {
 
 }
 
+trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] {
+  def newBuilder[A](): Builder[A, CC[A]]
+}
+
 object IterableFactory {
   import scala.language.implicitConversions
 

src/main/scala/strawman/collection/SortedSet.scala

@@ -13,6 +13,8 @@ trait SortedSetOps[A, +CC[X], +C <: SortedSet[A]]
 
   protected[this] def sortedFromIterable[B: Ordering](it: Iterable[B]): CC[B]
 
+  def iterableFactory: IterableFactory[Set]
+
   def firstKey: A = head
   def lastKey: A = last
 

src/main/scala/strawman/collection/immutable/LazyList.scala

@@ -65,7 +65,5 @@ object LazyList extends IterableFactory[LazyList] {
   def fromIterator[A](it: Iterator[A]): LazyList[A] =
     new LazyList(if (it.hasNext) Some(it.next(), fromIterator(it)) else None)
 
-  def newBuilder[A]: Builder[A, LazyList[A]] = ???
-
   def empty[A]: LazyList[A] = new LazyList[A](None)
 }

src/main/scala/strawman/collection/immutable/List.scala

@@ -55,14 +55,14 @@ case object Nil extends List[Nothing] {
   override def tail: Nothing = throw new UnsupportedOperationException("tail of empty list")
 }
 
-object List extends IterableFactory[List] {
+object List extends IterableFactoryWithBuilder[List] {
 
   def fromIterable[B](coll: collection.Iterable[B]): List[B] = coll match {
     case coll: List[B] => coll
     case _ => ListBuffer.fromIterable(coll).toList
   }
 
-  def newBuilder[A]: Builder[A, List[A]] = new ListBuffer[A].mapResult(_.toList)
+  def newBuilder[A](): Builder[A, List[A]] = new ListBuffer[A].mapResult(_.toList)
 
   def empty[A]: List[A] = Nil
 }

src/main/scala/strawman/collection/immutable/SortedMap.scala

@@ -13,12 +13,21 @@ trait SortedMapOps[K, +V, +CC[X, +Y] <: SortedMap[X, Y] with SortedMapOps[X, Y,
   extends MapOps[K, V, Map, C]
      with collection.SortedMapOps[K, V, CC, C] {
 
+    protected[this] def coll: CC[K, V]
+
     protected def mapFromIterable[K2, V2](it: collection.Iterable[(K2, V2)]): Map[K2, V2] =
       Map.fromIterable(it)
 
     // We override these methods to fix their return type (which would be `Map` otherwise)
     def updated[V1 >: V](key: K, value: V1): CC[K, V1]
     override def + [V1 >: V](kv: (K, V1)): CC[K, V1] = updated(kv._1, kv._2)
 
+    override def concat[V2 >: V](xs: collection.Iterable[(K, V2)]): CC[K, V2] = {
+        var result: CC[K, V2] = coll
+        val it = xs.iterator()
+        while (it.hasNext) result = result + it.next()
+        result
+    }
+
 }
 

src/main/scala/strawman/collection/mutable/ArrayBuffer.scala

@@ -124,7 +124,7 @@ class ArrayBuffer[A] private (initElems: Array[AnyRef], initLength: Int)
   override def className = "ArrayBuffer"
 }
 
-object ArrayBuffer extends IterableFactory[ArrayBuffer] {
+object ArrayBuffer extends IterableFactoryWithBuilder[ArrayBuffer] {
 
   /** Avoid reallocation of buffer if length is known. */
   def fromIterable[B](coll: collection.Iterable[B]): ArrayBuffer[B] =
@@ -136,9 +136,9 @@ object ArrayBuffer extends IterableFactory[ArrayBuffer] {
     }
     else new ArrayBuffer[B] ++= coll
 
-  def newBuilder[A]: Builder[A, ArrayBuffer[A]] = new ArrayBuffer[A]()
+  def newBuilder[A](): Builder[A, ArrayBuffer[A]] = new ArrayBuffer[A]()
 
-  def empty[A <: Any]: ArrayBuffer[A] = new ArrayBuffer[A]()
+  def empty[A]: ArrayBuffer[A] = new ArrayBuffer[A]()
 }
 
 class ArrayBufferView[A](val array: Array[AnyRef], val length: Int) extends IndexedView[A] {

src/main/scala/strawman/collection/mutable/HashSet.scala

@@ -35,7 +35,7 @@ final class HashSet[A](contents: FlatHashTable.Contents[A])
 
   protected[this] def fromSpecificIterable(coll: collection.Iterable[A]): HashSet[A] = fromIterable(coll)
 
-  protected[this] def newBuilder: Builder[A, HashSet[A]] = new GrowableBuilder[A, HashSet[A]](empty)
+  protected[this] def newBuilder: Builder[A, HashSet[A]] = HashSet.newBuilder[A]()
 
   def add(elem: A): this.type = {
     table.addElem(elem)
@@ -75,9 +75,12 @@ final class HashSet[A](contents: FlatHashTable.Contents[A])
 
 }
 
-object HashSet extends IterableFactory[HashSet] {
+object HashSet extends IterableFactoryWithBuilder[HashSet] {
 
   def fromIterable[B](it: strawman.collection.Iterable[B]): HashSet[B] = Growable.fromIterable(empty[B], it)
 
   def empty[A]: HashSet[A] = new HashSet[A]
+
+  def newBuilder[A](): Builder[A, HashSet[A]] = new GrowableBuilder[A, HashSet[A]](empty)
+
 }

src/main/scala/strawman/collection/mutable/ListBuffer.scala

@@ -200,11 +200,11 @@ class ListBuffer[A]
   override def className = "ListBuffer"
 }
 
-object ListBuffer extends IterableFactory[ListBuffer] {
+object ListBuffer extends IterableFactoryWithBuilder[ListBuffer] {
 
   def fromIterable[A](coll: collection.Iterable[A]): ListBuffer[A] = new ListBuffer[A] ++= coll
 
-  def newBuilder[A]: Builder[A, ListBuffer[A]] = new ListBuffer[A]
+  def newBuilder[A](): Builder[A, ListBuffer[A]] = new ListBuffer[A]
 
   def empty[A]: ListBuffer[A] = new ListBuffer[A]
 }

src/test/scala/strawman/collection/test/TraverseTest.scala

@@ -4,7 +4,7 @@ package collection.test
 
 import org.junit.Test
 import strawman.collection.Iterable
-import strawman.collection.mutable.{Builder, ArrayBuffer}
+import strawman.collection.mutable.{ArrayBuffer, Builder, Growable}
 import strawman.collection._
 
 import scala.{Any, Either, Int, Left, None, Option, Right, Some, Unit}
@@ -14,67 +14,40 @@ import java.lang.String
 
 class TraverseTest {
 
-  // You can either overload methods for PolyBuildable and OrderedPolyBuildable (if you want to support ordered collection types)
-  def optionSequence1[C[X] <: Iterable[X] with IterableOps[X, C, _], A](xs: C[Option[A]]): Option[C[A]] =
-    xs.foldLeft[Option[ArrayBuffer[A]]](Some(new ArrayBuffer[A])) { (bo, xo) =>
+  def optionSequence[CC[X] <: Iterable[X] with IterableOps[X, CC, _], A](xs: CC[Option[A]]): Option[CC[A]] = {
+    def folder[F[X] <: Growable[X]]: (Option[F[A]], Option[A]) => Option[F[A]] = { (bo, xo) =>
       (bo, xo) match {
         case (Some(builder), Some(a)) => Some(builder += a)
         case _ => None
       }
-    }.map(_.to(xs.iterableFactory))
-
-//  def optionSequence1[CC[_], A](xs: OrderedPolyBuildable[Option[A], CC] with Iterable[Option[A]])(implicit ev: Ordering[A]): Option[CC[A]] =
-//    xs.foldLeft[Option[Builder[A, CC[A]]]](Some(xs.newOrderedBuilder)) {
-//      case (Some(builder), Some(a)) => Some(builder += a)
-//      case _ => None
-//    }.map(_.result)
-//
-//  // ...or use BuildFrom to abstract over both and also allow building arbitrary collection types
-//  def optionSequence[CC[X] <: Iterable[X], A](xs: CC[Option[A]])(implicit bf: BuildFrom[CC[Option[A]], A]): Option[bf.To] =
-//    xs.foldLeft[Option[Builder[A, bf.To]]](Some(bf.newBuilder(xs))) {
-//      case (Some(builder), Some(a)) => Some(builder += a)
-//      case _ => None
-//    }.map(_.result)
-//
-//  def eitherSequence[C[X] <: Iterable[X], A, B](xs: C[Either[A, B]])(implicit bf: BuildFrom[xs.type, B]): Either[A, bf.To] =
-//    xs.foldLeft[Either[A, Builder[B, bf.To]]](Right(bf.newBuilder(xs))) {
-//      case (Right(builder), Right(b)) => Right(builder += b)
-//      case (Left(a)       ,        _) => Left(a)
-//      case (_             ,  Left(a)) => Left(a)
-//    }.right.map(_.result)
+    }
+    val factory = xs.iterableFactory
+    factory match {
+      case iterableBuilder: IterableFactoryWithBuilder[CC] =>
+        xs.foldLeft[Option[Builder[A, CC[A]]]](
+          Some(iterableBuilder.newBuilder[A]())
+        )(
+          folder[({ type l[X] = Builder[X, CC[X]] })#l]
+        ).map(_.result)
+      case _ =>
+        xs.foldLeft[Option[ArrayBuffer[A]]](Some(new ArrayBuffer[A]))(folder).map(_.to(xs.iterableFactory))
+    }
+  }
 
   @Test
   def optionSequence1Test: Unit = {
     val xs1 = immutable.List(Some(1), None, Some(2))
-    val o1 = optionSequence1(xs1)
+    val o1 = optionSequence(xs1)
     val o1t: Option[immutable.List[Int]] = o1
 
     val xs2: immutable.Set[Option[String]] = immutable.TreeSet(Some("foo"), Some("bar"), None)
-    val o2 = optionSequence1(xs2)
+    val o2 = optionSequence(xs2)
     val o2t: Option[immutable.Set[String]] = o2
-  }
 
-//  def optionSequenceTest: Unit = {
-//    val xs1 = immutable.List(Some(1), None, Some(2))
-//    val o1 = optionSequence(xs1)
-//    val o1t: Option[immutable.List[Int]] = o1
-//
-//    val xs2 = immutable.TreeSet(Some("foo"), Some("bar"), None)
-//    val o2 = optionSequence(xs2)
-//    val o2t: Option[immutable.TreeSet[String]] = o2
-//
-//    // Breakout-like use case from https://github.com/scala/scala/pull/5233:
-//    val xs4 = immutable.List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
-//    val o4 = optionSequence(xs4)(immutable.TreeMap) // same syntax as in `.to`
-//    val o4t: Option[immutable.TreeMap[Int, String]] = o4
-//  }
-//
-//  @Test
-//  def eitherSequenceTest: Unit = {
-//    val xs3 = mutable.ListBuffer(Right("foo"), Left(0), Right("bar"))
-//    val xs3t: mutable.ListBuffer[Either[Int, String]] = xs3
-//    val e1 = eitherSequence(xs3)
-//    val e1t: Either[Int, mutable.ListBuffer[String]] = e1
-//  }
+    val xs4 = immutable.List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
+    val o4 = optionSequence(xs4)
+    val o4t: Option[immutable.List[(Int, String)]] = o4
+    val o5: Option[immutable.TreeMap[Int, String]] = o4.map(_.to(immutable.TreeMap))
+  }
 
 }