Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.

Pull up the newBuilder method from Buildable to IterableOps. #110

Merged
merged 2 commits into from
Jun 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/main/scala/strawman/collection/ArrayOps.scala
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
package strawman
package collection

import scala.{Array, Char, Int, AnyVal}
import scala.{AnyVal, Array, Char, Int}
import scala.Predef.???
import mutable.ArrayBuffer
import mutable.{ArrayBuffer, GrowableBuilder}

import scala.reflect.ClassTag

class ArrayOps[A](val xs: Array[A])
extends AnyVal
with SeqOps[A, immutable.Seq, Array[A]] // should be IndexedSeq once we have an instance type
with Buildable[A, Array[A]]
with ArrayLike[A] {
with SeqOps[A, immutable.IndexedSeq, Array[A]]
with StrictOptimizedIterableOps[A, Array[A]]
with ArrayLike[A] {

protected def coll = new ArrayView(xs)
protected[this] def coll = new ArrayView(xs)

def length = xs.length
def apply(i: Int) = xs.apply(i)
Expand All @@ -21,12 +22,12 @@ class ArrayOps[A](val xs: Array[A])

def elemTag: ClassTag[A] = ClassTag(xs.getClass.getComponentType)

def iterableFactory = immutable.Seq
def iterableFactory = immutable.IndexedSeq

protected[this] def fromTaggedIterable[B: ClassTag](coll: Iterable[B]): Array[B] = coll.toArray[B]
protected[this] def fromSpecificIterable(coll: Iterable[A]): Array[A] = coll.toArray[A](elemTag)

protected[this] def newBuilder = new ArrayBuffer[A].mapResult(_.toArray(elemTag))
protected[this] def newSpecificBuilder() = new GrowableBuilder(ArrayBuffer.empty[A]).mapResult(_.toArray(elemTag))

override def knownSize = xs.length

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/BitSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ trait BitSetOps[+C <: BitSet with BitSetOps[C]]
extends SortedSetOps[Int, SortedSet, C] {
import BitSetOps._

protected def coll: C
protected[this] def coll: C

final def ordering: Ordering[Int] = Ordering.Int

Expand Down
68 changes: 59 additions & 9 deletions src/main/scala/strawman/collection/Factories.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ trait IterableFactory[+CC[_]] {

}

trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] {
def newBuilder[A](): Builder[A, CC[A]]
}

object IterableFactory {
import scala.language.implicitConversions

Expand All @@ -47,6 +43,18 @@ object IterableFactory {

}

trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would you think of StrictIterableFactory instead of IterableFactoryWithBuilder?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

def newBuilder[A](): Builder[A, CC[A]]
}

object IterableFactoryWithBuilder {
class Delegate[CC[_]](delegate: IterableFactoryWithBuilder[CC])
extends IterableFactory.Delegate[CC](delegate)
with IterableFactoryWithBuilder[CC] {
def newBuilder[A](): Builder[A, CC[A]] = delegate.newBuilder()
}
}

trait SpecificIterableFactory[-A, +C] extends FromSpecificIterable[A, C] {
def empty: C

Expand All @@ -55,8 +63,12 @@ trait SpecificIterableFactory[-A, +C] extends FromSpecificIterable[A, C] {
def fill(n: Int)(elem: => A): C = fromSpecificIterable(View.Fill(n)(elem))
}

trait SpecificIterableFactoryWithBuilder[-A, +C] extends SpecificIterableFactory[A, C] {
def newBuilder(): Builder[A, C]
}

/** Factory methods for collections of kind `* −> * -> *` */
trait MapFactory[+CC[X, Y]] {
trait MapFactory[+CC[_, _]] {

def empty[K, V]: CC[K, V]
def fromIterable[K, V](it: Iterable[(K, V)]): CC[K, V]
Expand All @@ -67,14 +79,28 @@ trait MapFactory[+CC[X, Y]] {
object MapFactory {
import scala.language.implicitConversions

implicit def toSpecific[K, V, CC[X, Y]](factory: MapFactory[CC]): FromSpecificIterable[(K, V), CC[K, V]] =
implicit def toSpecific[K, V, CC[_, _]](factory: MapFactory[CC]): FromSpecificIterable[(K, V), CC[K, V]] =
new FromSpecificIterable[(K, V), CC[K, V]] {
def fromSpecificIterable(it: Iterable[(K, V)]): CC[K, V] = factory.fromIterable[K, V](it)
}

class Delegate[C[X, Y]](delegate: MapFactory[C]) extends MapFactory[C] {
def fromIterable[K, V](it: Iterable[(K, V)]): C[K, V] = delegate.fromIterable(it)
def empty[K, V]: C[K, V] = delegate.empty
class Delegate[CC[_, _]](delegate: MapFactory[CC]) extends MapFactory[CC] {
def fromIterable[K, V](it: Iterable[(K, V)]): CC[K, V] = delegate.fromIterable(it)
def empty[K, V]: CC[K, V] = delegate.empty
}

}

trait MapFactoryWithBuilder[+CC[_, _]] extends MapFactory[CC] {
def newBuilder[K, V](): Builder[(K, V), CC[K, V]]
}

object MapFactoryWithBuilder {

class Delegate[CC[_, _]](delegate: MapFactoryWithBuilder[CC])
extends MapFactory.Delegate[CC](delegate)
with MapFactoryWithBuilder[CC] {
def newBuilder[K, V](): Builder[(K, V), CC[K, V]] = delegate.newBuilder()
}

}
Expand Down Expand Up @@ -106,6 +132,18 @@ object SortedIterableFactory {

}

trait SortedIterableFactoryWithBuilder[+CC[_]] extends SortedIterableFactory[CC] {
def newBuilder[A : Ordering](): Builder[A, CC[A]]
}

object SortedIterableFactoryWithBuilder {
class Delegate[CC[_]](delegate: SortedIterableFactoryWithBuilder[CC])
extends SortedIterableFactory.Delegate[CC](delegate)
with SortedIterableFactoryWithBuilder[CC] {
def newBuilder[A: Ordering](): Builder[A, CC[A]] = delegate.newBuilder()
}
}

/** Factory methods for collections of kind `* −> * -> *` which require an implicit evidence value for the key type */
trait SortedMapFactory[+CC[X, Y]] {

Expand All @@ -131,3 +169,15 @@ object SortedMapFactory {
}

}

trait SortedMapFactoryWithBuilder[+CC[_, _]] extends SortedMapFactory[CC] {
def newBuilder[K : Ordering, V](): Builder[(K, V), CC[K, V]]
}

object SortedMapFactoryWithBuilder {
class Delegate[CC[_, _]](delegate: SortedMapFactoryWithBuilder[CC])
extends SortedMapFactory.Delegate[CC](delegate)
with SortedMapFactoryWithBuilder[CC] {
def newBuilder[K: Ordering, V](): Builder[(K, V), CC[K, V]] = delegate.newBuilder()
}
}
41 changes: 15 additions & 26 deletions src/main/scala/strawman/collection/Iterable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import java.lang.String
trait Iterable[+A] extends IterableOnce[A] with IterableOps[A, Iterable, Iterable[A]] {

/** The collection itself */
protected def coll: this.type = this
protected[this] def coll: this.type = this

}

/** Base trait for Iterable operations
Expand All @@ -28,13 +29,23 @@ trait Iterable[+A] extends IterableOnce[A] with IterableOps[A, Iterable, Iterabl
*/
trait IterableOps[+A, +CC[X], +C] extends Any {

protected def coll: Iterable[A]
protected[this] def coll: Iterable[A]

protected[this] def fromSpecificIterable(coll: Iterable[A]): C

protected[this] def fromIterable[E](it: Iterable[E]): CC[E] = iterableFactory.fromIterable(it)

def iterableFactory: IterableFactory[CC]

protected[this] def fromIterable[E](it: Iterable[E]): CC[E] = iterableFactory.fromIterable(it)
/**
* @return a strict builder for the same collection type.
*
* Note that in the case of lazy collections (e.g. [[View]] or [[immutable.LazyList]]),
* it is possible to implement this method but the resulting `Builder` will break laziness.
* As a consequence, operations should preferably be implemented on top of views rather
* than builders.
*/
protected[this] def newSpecificBuilder(): Builder[A, C]

/** Apply `f` to each element for its side effects
* Note: [U] parameter needed to help scalac's type inference.
Expand Down Expand Up @@ -93,7 +104,7 @@ trait IterableOps[+A, +CC[X], +C] extends Any {
* xs.to(ArrayBuffer)
* xs.to(BitSet) // for xs: Iterable[Int]
*/
def to[C](f: FromSpecificIterable[A, C]): C = f.fromSpecificIterable(coll)
def to[C1](f: FromSpecificIterable[A, C1]): C1 = f.fromSpecificIterable(coll)

/** Convert collection to array. */
def toArray[B >: A: ClassTag]: Array[B] =
Expand Down Expand Up @@ -251,25 +262,3 @@ trait IterableOps[+A, +CC[X], +C] extends Any {
def zip[B](xs: IterableOnce[B]): CC[(A @uncheckedVariance, B)] = fromIterable(View.Zip(coll, xs))
// sound bcs of VarianceNote
}

/** Base trait for strict collections that can be built using a builder.
* @tparam A the element type of the collection
* @tparam C the type of the underlying collection
*/
trait Buildable[+A, +C] extends Any with IterableOps[A, AnyConstr, C] {

/** Creates a new builder. */
protected[this] def newBuilder: Builder[A, C]

/** Optimized, push-based version of `partition`. */
override def partition(p: A => Boolean): (C, C) = {
val l, r = newBuilder
coll.iterator().foreach(x => (if (p(x)) l else r) += x)
(l.result(), r.result())
}

// one might also override other transforms here to avoid generating
// iterators if it helps efficiency.
}


4 changes: 2 additions & 2 deletions src/main/scala/strawman/collection/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ trait MapOps[K, +V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]]
extends IterableOps[(K, V), Iterable, C]
with PartialFunction[K, V] {

protected def coll: Map[K, V]
protected[this] def coll: Map[K, V]

/** Similar to fromIterable, but returns a Map collection type */
protected[this] def mapFromIterable[K2, V2](it: Iterable[(K2, V2)]): CC[K2, V2]
Expand Down Expand Up @@ -85,4 +85,4 @@ trait MapOps[K, +V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]]

}

object Map extends MapFactory.Delegate[Map](immutable.Map)
object Map extends MapFactoryWithBuilder.Delegate[Map](immutable.Map)
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/Seq.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ trait IndexedSeqOps[+A, +CC[X] <: IndexedSeq[X], +C] extends Any with SeqOps[A,
/** Base trait for linear Seq operations */
trait LinearSeqOps[+A, +CC[X] <: LinearSeq[X], +C <: LinearSeq[A]] extends Any with SeqOps[A, CC, C] {

protected def coll: Seq[A]
protected[this] def coll: Seq[A]

/** To be overridden in implementations: */
def isEmpty: Boolean
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/Set.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ trait SetOps[A, +CC[X], +C <: Set[A]]
with (A => Boolean)
with Equals {

protected def coll: C
protected[this] def coll: C

def contains(elem: A): Boolean

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/SortedMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,4 @@ trait SortedMapOps[K, +V, +CC[X, Y] <: SortedMap[X, Y] with SortedMapOps[X, Y, C

}

object SortedMap extends SortedMapFactory.Delegate[SortedMap](TreeMap)
object SortedMap extends SortedMapFactoryWithBuilder.Delegate[SortedMap](TreeMap)
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/SortedSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ trait SortedSetOps[A, +CC[X], +C <: SortedSet[A]]
)
}

object SortedSet extends SortedIterableFactory.Delegate[SortedSet](immutable.SortedSet)
object SortedSet extends SortedIterableFactoryWithBuilder.Delegate[SortedSet](immutable.SortedSet)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package strawman
package collection

import scala.{Any, Boolean}

/**
* Trait that overrides operations to take advantage of strict builders.
*
* @tparam A Elements type
* @tparam C Collection type
*/
trait StrictOptimizedIterableOps[+A, +C]
extends Any
with IterableOps[A, AnyConstr, C] {

/** Optimized, push-based version of `partition`. */
override def partition(p: A => Boolean): (C, C) = {
val l, r = newSpecificBuilder()
coll.iterator().foreach(x => (if (p(x)) l else r) += x)
(l.result(), r.result())
}

// one might also override other transforms here to avoid generating
// iterators if it helps efficiency.

}
6 changes: 3 additions & 3 deletions src/main/scala/strawman/collection/StringOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import scala.reflect.ClassTag
class StringOps(val s: String)
extends AnyVal
with SeqOps[Char, Seq, String]
with Buildable[Char, String]
with StrictOptimizedIterableOps[Char, String]
with ArrayLike[Char] {

protected def coll = new StringView(s)
protected[this] def coll = new StringView(s)

protected[this] def fromSpecificIterable(coll: Iterable[Char]): String = {
val sb = new StringBuilder
Expand All @@ -24,7 +24,7 @@ class StringOps(val s: String)

def iterableFactory = List

protected[this] def newBuilder = new StringBuilder
protected[this] def newSpecificBuilder() = new StringBuilder

def length = s.length
def apply(i: Int) = s.charAt(i)
Expand Down
8 changes: 6 additions & 2 deletions src/main/scala/strawman/collection/View.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package strawman.collection

import strawman.collection.mutable.Builder

import scala.{Any, Boolean, Equals, Int, Nothing, annotation}
import scala.Predef.intWrapper

Expand All @@ -9,8 +11,10 @@ trait View[+A] extends Iterable[A] with IterableOps[A, View, View[A]] {

def iterableFactory = View

override protected[this] def fromSpecificIterable(coll: Iterable[A]): View[A] =
fromIterable(coll)
protected[this] def fromSpecificIterable(coll: Iterable[A]): View[A] = fromIterable(coll)

protected[this] def newSpecificBuilder(): Builder[A, View[A]] =
immutable.IndexedSeq.newBuilder().mapResult(_.view)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how newSpecificBuilder() is implemented in View.

Note that (fortunately) the View factory object does not publicly expose this builder.


override def className = "View"
}
Expand Down
10 changes: 7 additions & 3 deletions src/main/scala/strawman/collection/immutable/BitSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package collection
package immutable

import BitSetOps.{LogWL, updateArray}
import mutable.Builder
import mutable.{Builder, GrowableBuilder}

import scala.{Array, Boolean, Int, Long, Ordering, SerialVersionUID, Serializable, Unit}
import scala.Predef.require
Expand All @@ -22,6 +22,7 @@ sealed abstract class BitSet
with collection.BitSet
with SortedSetOps[Int, SortedSet, BitSet]
with collection.BitSetOps[BitSet]
with StrictOptimizedIterableOps[Int, BitSet]
with Serializable {

def empty: BitSet = BitSet.empty
Expand All @@ -30,7 +31,7 @@ sealed abstract class BitSet

protected[this] def fromSpecificIterable(coll: collection.Iterable[Int]): BitSet = BitSet.fromSpecificIterable(coll)
protected[this] def sortedFromIterable[B : Ordering](it: collection.Iterable[B]): SortedSet[B] = SortedSet.sortedFromIterable(it)

protected[this] def newSpecificBuilder(): Builder[Int, BitSet] = BitSet.newBuilder()

protected[collection] def fromBitMaskNoCopy(elems: Array[Long]): BitSet = BitSet.fromBitMaskNoCopy(elems)

Expand Down Expand Up @@ -58,7 +59,7 @@ sealed abstract class BitSet
protected def updateWord(idx: Int, w: Long): BitSet
}

object BitSet extends SpecificIterableFactory[Int, BitSet] {
object BitSet extends SpecificIterableFactoryWithBuilder[Int, BitSet] {

def fromSpecificIterable(it: strawman.collection.Iterable[Int]): BitSet =
it match {
Expand Down Expand Up @@ -93,6 +94,9 @@ object BitSet extends SpecificIterableFactory[Int, BitSet] {

def empty: BitSet = new BitSet1(0L)

def newBuilder(): Builder[Int, BitSet] =
new GrowableBuilder(mutable.BitSet.empty).mapResult(bs => fromBitMaskNoCopy(bs.elems))

@SerialVersionUID(2260107458435649300L)
class BitSet1(val elems: Long) extends BitSet {
protected[collection] def nwords = 1
Expand Down
Loading