From e857b3fb2aa891a3e5b6ca34a1b054434fc84945 Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Mon, 19 Dec 2016 13:11:38 +0100 Subject: [PATCH] Make it clearer that the `iterator` method will return a fresh new Iterator at each call. --- .../collection/CollectionStrawman6.scala | 64 +++++++++---------- src/main/scala/strawman/collection/View.scala | 42 ++++++------ .../scala/strawman/collection/package.scala | 6 +- .../scala/strawman/collection/test/Test.scala | 6 +- 4 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/main/scala/strawman/collection/CollectionStrawman6.scala b/src/main/scala/strawman/collection/CollectionStrawman6.scala index 034da15f59..b77a4831df 100644 --- a/src/main/scala/strawman/collection/CollectionStrawman6.scala +++ b/src/main/scala/strawman/collection/CollectionStrawman6.scala @@ -4,12 +4,13 @@ import Predef.{augmentString => _, wrapString => _, _} import scala.reflect.ClassTag import annotation.unchecked.uncheckedVariance import annotation.tailrec +import scala.language.higherKinds /* ------------ Base Traits -------------------------------- */ /** Iterator can be used only once */ trait IterableOnce[+A] { - def iterator: Iterator[A] + def iterator(): Iterator[A] } /** Base trait for instances that can construct a collection from an iterable */ @@ -50,14 +51,14 @@ trait LinearSeq[+A] extends Seq[A] with LinearSeqLike[A, LinearSeq] { self => def tail: LinearSeq[A] /** `iterator` is overridden in terms of `head` and `tail` */ - def iterator = new Iterator[A] { + def iterator() = new Iterator[A] { private[this] var current: Seq[A] = self def hasNext = !current.isEmpty - def next = { val r = current.head; current = current.tail; r } + def next() = { val r = current.head; current = current.tail; r } } /** `length` is defined in terms of `iterator` */ - def length: Int = iterator.length + def length: Int = iterator().length /** `apply` is defined in terms of `drop`, which is in turn defined in * terms of `tail`. @@ -78,8 +79,8 @@ trait IndexedSeq[+A] extends Seq[A] { self => } /** Base trait for strict collections that can be built using a builder. - * @param A the element type of the collection - * @param Repr the type of the underlying collection + * @tparam A the element type of the collection + * @tparam Repr the type of the underlying collection */ trait Buildable[+A, +Repr] extends Any with IterableMonoTransforms[A, Repr] { @@ -89,7 +90,7 @@ trait Buildable[+A, +Repr] extends Any with IterableMonoTransforms[A, Repr] { /** Optimized, push-based version of `partition`. */ override def partition(p: A => Boolean): (Repr, Repr) = { val l, r = newBuilder - coll.iterator.foreach(x => (if (p(x)) l else r) += x) + coll.iterator().foreach(x => (if (p(x)) l else r) += x) (l.result, r.result) } @@ -108,7 +109,7 @@ trait Builder[-A, +To] { self => /** Bulk append. Can be overridden if specialized implementations are available. */ def ++=(xs: IterableOnce[A]): this.type = { - xs.iterator.foreach(+=) + xs.iterator().foreach(+=) this } @@ -177,25 +178,25 @@ trait LinearSeqLike[+A, +C[X] <: LinearSeq[X]] extends SeqLike[A, C] { */ trait IterableOps[+A] extends Any { protected def coll: Iterable[A] - private def iterator = coll.iterator + private def iterator() = coll.iterator() /** Apply `f` to each element for tis side effects */ - def foreach(f: A => Unit): Unit = iterator.foreach(f) + def foreach(f: A => Unit): Unit = iterator().foreach(f) /** Fold left */ - def foldLeft[B](z: B)(op: (B, A) => B): B = iterator.foldLeft(z)(op) + def foldLeft[B](z: B)(op: (B, A) => B): B = iterator().foldLeft(z)(op) /** Fold right */ - def foldRight[B](z: B)(op: (A, B) => B): B = iterator.foldRight(z)(op) + def foldRight[B](z: B)(op: (A, B) => B): B = iterator().foldRight(z)(op) /** The index of the first element in this collection for which `p` holds. */ - def indexWhere(p: A => Boolean): Int = iterator.indexWhere(p) + def indexWhere(p: A => Boolean): Int = iterator().indexWhere(p) /** Is the collection empty? */ - def isEmpty: Boolean = !iterator.hasNext + def isEmpty: Boolean = !iterator().hasNext /** The first element of the collection. */ - def head: A = iterator.next() + def head: A = iterator().next() /** The number of elements in this collection, if it can be cheaply computed, * -1 otherwise. Cheaply usually means: Not requiring a collection traversal. @@ -205,10 +206,10 @@ trait IterableOps[+A] extends Any { /** The number of elements in this collection. Does not terminate for * infinite collections. */ - def size: Int = if (knownSize >= 0) knownSize else iterator.length + def size: Int = if (knownSize >= 0) knownSize else iterator().length /** A view representing the elements of this collection. */ - def view: View[A] = View.fromIterator(iterator) + def view: View[A] = View.fromIterator(iterator()) /** Given a collection factory `fi` for collections of type constructor `C`, * convert this collection to one of type `C[A]`. Example uses: @@ -229,7 +230,7 @@ trait IterableOps[+A] extends Any { /** Copy all elements of this collection to array `xs`, starting at `start`. */ def copyToArray[B >: A](xs: Array[B], start: Int = 0): xs.type = { var i = start - val it = iterator + val it = iterator() while (it.hasNext) { xs(i) = it.next() i += 1 @@ -320,7 +321,7 @@ trait SeqMonoTransforms[+A, +Repr] extends Any with IterableMonoTransforms[A, Re case v: IndexedView[A] => fromIterableWithSameElemType(v.reverse) case _ => var xs: List[A] = Nil - var it = coll.iterator + var it = coll.iterator() while (it.hasNext) xs = it.next() :: xs fromIterableWithSameElemType(xs) } @@ -386,7 +387,7 @@ extends Seq[A] private var aliased = false private var len = 0 - def iterator = first.iterator + def iterator() = first.iterator() def fromIterable[B](coll: Iterable[B]) = ListBuffer.fromIterable(coll) @@ -451,7 +452,7 @@ extends Seq[A] override def view = new ArrayBufferView(elems, start, end) - def iterator = view.iterator + def iterator() = view.iterator() def fromIterable[B](it: Iterable[B]): ArrayBuffer[B] = ArrayBuffer.fromIterable(it) @@ -506,7 +507,7 @@ object ArrayBuffer extends IterableFactory[ArrayBuffer] { def fromIterable[B](coll: Iterable[B]): ArrayBuffer[B] = if (coll.knownSize >= 0) { val elems = new Array[AnyRef](coll.knownSize) - val it = coll.iterator + val it = coll.iterator() for (i <- 0 until elems.length) elems(i) = it.next().asInstanceOf[AnyRef] new ArrayBuffer[B](elems, elems.length) } @@ -563,7 +564,7 @@ object LazyList extends IterableFactory[LazyList] { def fromIterable[B](coll: Iterable[B]): LazyList[B] = coll match { case coll: LazyList[B] => coll - case _ => fromIterator(coll.iterator) + case _ => fromIterator(coll.iterator()) } def fromIterator[B](it: Iterator[B]): LazyList[B] = @@ -600,12 +601,11 @@ case class ArrayView[A](xs: Array[A]) extends IndexedView[A] { /* ---------- Iterators ---------------------------------------------------*/ /** A core Iterator class */ -trait Iterator[+A] extends IterableOnce[A] { self => +trait Iterator[+A] { self => def hasNext: Boolean def next(): A - def iterator = this def foldLeft[B](z: B)(op: (B, A) => B): B = - if (hasNext) foldLeft(op(z, next))(op) else z + if (hasNext) foldLeft(op(z, next()))(op) else z def foldRight[B](z: B)(op: (A, B) => B): B = if (hasNext) op(next(), foldRight(z)(op)) else z def foreach(f: A => Unit): Unit = @@ -652,7 +652,7 @@ trait Iterator[+A] extends IterableOnce[A] { self => private var myCurrent: Iterator[B] = Iterator.empty private def current = { while (!myCurrent.hasNext && self.hasNext) - myCurrent = f(self.next()).iterator + myCurrent = f(self.next()).iterator() myCurrent } def hasNext = current.hasNext @@ -663,7 +663,7 @@ trait Iterator[+A] extends IterableOnce[A] { self => private var first = true private def current = { if (!myCurrent.hasNext && first) { - myCurrent = xs.iterator + myCurrent = xs.iterator() first = false } myCurrent @@ -674,7 +674,7 @@ trait Iterator[+A] extends IterableOnce[A] { self => def take(n: Int): Iterator[A] = new Iterator[A] { private var i = 0 def hasNext = self.hasNext && i < n - def next = + def next() = if (hasNext) { i += 1 self.next() @@ -690,7 +690,7 @@ trait Iterator[+A] extends IterableOnce[A] { self => this } def zip[B](that: IterableOnce[B]): Iterator[(A, B)] = new Iterator[(A, B)] { - val thatIterator = that.iterator + val thatIterator = that.iterator() def hasNext = self.hasNext && thatIterator.hasNext def next() = (self.next(), thatIterator.next()) } @@ -699,10 +699,10 @@ trait Iterator[+A] extends IterableOnce[A] { self => object Iterator { val empty: Iterator[Nothing] = new Iterator[Nothing] { def hasNext = false - def next = throw new NoSuchElementException("next on empty iterator") + def next() = throw new NoSuchElementException("next on empty iterator") } def apply[A](xs: A*): Iterator[A] = new IndexedView[A] { val length = xs.length def apply(n: Int) = xs(n) - }.iterator + }.iterator() } diff --git a/src/main/scala/strawman/collection/View.scala b/src/main/scala/strawman/collection/View.scala index 177a2f2245..e6258e7098 100644 --- a/src/main/scala/strawman/collection/View.scala +++ b/src/main/scala/strawman/collection/View.scala @@ -12,7 +12,7 @@ trait View[+A] extends Iterable[A] with IterableLike[A, View] { /** Avoid copying if source collection is already a view. */ override def fromIterable[B](c: Iterable[B]): View[B] = c match { case c: View[B] => c - case _ => View.fromIterator(c.iterator) + case _ => View.fromIterator(c.iterator()) } override def className = "View" } @@ -20,28 +20,28 @@ trait View[+A] extends Iterable[A] with IterableLike[A, View] { /** This object reifies operations on views as case classes */ object View { def fromIterator[A](it: => Iterator[A]): View[A] = new View[A] { - def iterator = it + def iterator() = it } /** The empty view */ case object Empty extends View[Nothing] { - def iterator = Iterator.empty + def iterator() = Iterator.empty override def knownSize = 0 } /** A view with given elements */ case class Elems[A](xs: A*) extends View[A] { - def iterator = Iterator(xs: _*) + def iterator() = Iterator(xs: _*) override def knownSize = xs.length // should be: xs.knownSize, but A*'s are not sequences in this strawman. } /** A view that filters an underlying collection. */ - case class Filter[A](val underlying: Iterable[A], p: A => Boolean) extends View[A] { - def iterator = underlying.iterator.filter(p) + case class Filter[A](underlying: Iterable[A], p: A => Boolean) extends View[A] { + def iterator() = underlying.iterator().filter(p) } /** A view that partitions an underlying collection into two views */ - case class Partition[A](val underlying: Iterable[A], p: A => Boolean) { + case class Partition[A](underlying: Iterable[A], p: A => Boolean) { /** The view consisting of all elements of the underlying collection * that satisfy `p`. @@ -56,12 +56,12 @@ object View { /** A view representing one half of a partition. */ case class Partitioned[A](partition: Partition[A], cond: Boolean) extends View[A] { - def iterator = partition.underlying.iterator.filter(x => partition.p(x) == cond) + def iterator() = partition.underlying.iterator().filter(x => partition.p(x) == cond) } /** A view that drops leading elements of the underlying collection. */ case class Drop[A](underlying: Iterable[A], n: Int) extends View[A] { - def iterator = underlying.iterator.drop(n) + def iterator() = underlying.iterator().drop(n) protected val normN = n max 0 override def knownSize = if (underlying.knownSize >= 0) (underlying.knownSize - normN) max 0 else -1 @@ -69,7 +69,7 @@ object View { /** A view that takes leading elements of the underlying collection. */ case class Take[A](underlying: Iterable[A], n: Int) extends View[A] { - def iterator = underlying.iterator.take(n) + def iterator() = underlying.iterator().take(n) protected val normN = n max 0 override def knownSize = if (underlying.knownSize >= 0) underlying.knownSize min normN else -1 @@ -77,20 +77,20 @@ object View { /** A view that maps elements of the underlying collection. */ case class Map[A, B](underlying: Iterable[A], f: A => B) extends View[B] { - def iterator = underlying.iterator.map(f) + def iterator() = underlying.iterator().map(f) override def knownSize = underlying.knownSize } /** A view that flatmaps elements of the underlying collection. */ case class FlatMap[A, B](underlying: Iterable[A], f: A => IterableOnce[B]) extends View[B] { - def iterator = underlying.iterator.flatMap(f) + def iterator() = underlying.iterator().flatMap(f) } /** A view that concatenates elements of the underlying collection with the elements * of another collection or iterator. */ case class Concat[A](underlying: Iterable[A], other: IterableOnce[A]) extends View[A] { - def iterator = underlying.iterator ++ other + def iterator() = underlying.iterator() ++ other override def knownSize = other match { case other: Iterable[_] if underlying.knownSize >= 0 && other.knownSize >= 0 => underlying.knownSize + other.knownSize @@ -103,12 +103,10 @@ object View { * of another collection or iterator. */ case class Zip[A, B](underlying: Iterable[A], other: IterableOnce[B]) extends View[(A, B)] { - def iterator = underlying.iterator.zip(other) + def iterator() = underlying.iterator().zip(other) override def knownSize = other match { - case other: Iterable[_] if underlying.knownSize >= 0 && other.knownSize >= 0 => - underlying.knownSize min other.knownSize - case _ => - -1 + case other: Iterable[_] => underlying.knownSize min other.knownSize + case _ => -1 } } } @@ -116,7 +114,7 @@ object View { /** View defined in terms of indexing a range */ trait IndexedView[+A] extends View[A] with ArrayLike[A] { self => - def iterator: Iterator[A] = new Iterator[A] { + def iterator(): Iterator[A] = new Iterator[A] { private var current = 0 def hasNext = current < self.length def next: A = { @@ -136,21 +134,21 @@ object IndexedView { class Take[A](underlying: IndexedView[A], n: Int) extends View.Take(underlying, n) with IndexedView[A] { - override def iterator = super.iterator // needed to avoid "conflicting overrides" error + override def iterator() = super.iterator() // needed to avoid "conflicting overrides" error def length = underlying.length min normN def apply(i: Int) = underlying.apply(i) } class Drop[A](underlying: IndexedView[A], n: Int) extends View.Take(underlying, n) with IndexedView[A] { - override def iterator = super.iterator + override def iterator() = super.iterator() def length = (underlying.length - normN) max 0 def apply(i: Int) = underlying.apply(i + normN) } class Map[A, B](underlying: IndexedView[A], f: A => B) extends View.Map(underlying, f) with IndexedView[B] { - override def iterator = super.iterator + override def iterator() = super.iterator() def length = underlying.length def apply(n: Int) = f(underlying.apply(n)) } diff --git a/src/main/scala/strawman/collection/package.scala b/src/main/scala/strawman/collection/package.scala index f28ffe64f2..a94d813e5f 100644 --- a/src/main/scala/strawman/collection/package.scala +++ b/src/main/scala/strawman/collection/package.scala @@ -93,7 +93,7 @@ package object collection extends LowPriority { with ArrayLike[Char] { protected def coll = new StringView(s) - def iterator = coll.iterator + def iterator() = coll.iterator() protected def fromIterableWithSameElemType(coll: Iterable[Char]): String = { val sb = new StringBuilder @@ -135,7 +135,7 @@ package object collection extends LowPriority { */ def ++(xs: IterableOnce[Char]): String = { val sb = new StringBuilder() ++= s - for (ch <- xs.iterator) sb += ch + for (ch <- xs.iterator()) sb += ch sb.result } @@ -152,7 +152,7 @@ package object collection extends LowPriority { with ArrayLike[A] { protected def coll = new ArrayView(xs) - def iterator = coll.iterator + def iterator() = coll.iterator() def length = xs.length def apply(i: Int) = xs.apply(i) diff --git a/src/test/scala/strawman/collection/test/Test.scala b/src/test/scala/strawman/collection/test/Test.scala index 237fe9558a..3c62d46113 100644 --- a/src/test/scala/strawman/collection/test/Test.scala +++ b/src/test/scala/strawman/collection/test/Test.scala @@ -121,9 +121,9 @@ class StrawmanTest { val ys7: String = xs7 val xs8 = xs.drop(2) val ys8: String = xs8 - val xs9 = xs.map(_ + 1) // !!! need a language change to make this work without the : Char + val xs9 = xs.map(_ + 1) val ys9: Seq[Int] = xs9 - val xs9a = xs.map(_.toUpper) // !!! need a language change to make this work without the : Char + val xs9a = xs.map(_.toUpper) val ys9a: String = xs9a val xs10 = xs.flatMap((x: Char) => s"$x,$x") val ys10: String = xs10 @@ -133,7 +133,7 @@ class StrawmanTest { val ys11a: String = xs11a val xs12 = xs ++ Nil val ys12: String = xs12 - val xs13 = Nil ++ xs.iterator + val xs13 = Nil ++ xs val ys13: List[Char] = xs13 val xs14 = xs ++ List("xyz") val ys14: Seq[Any] = xs14