Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.
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
25 changes: 23 additions & 2 deletions src/main/scala/strawman/collection/Iterable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,18 @@ trait IterableOps[+A, +CC[X], +C] extends Any {
def take(n: Int): C = fromSpecificIterable(View.Take(coll, n))

/** A collection containing the last `n` elements of this collection. */
def takeRight(n: Int): C = fromSpecificIterable(View.TakeRight(coll, n))
def takeRight(n: Int): C = {
val b = newSpecificBuilder()
b.sizeHintBounded(n, coll)
val lead = coll.iterator() drop n
val it = coll.iterator()
while (lead.hasNext) {
lead.next()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a cute algorithm, but I can't imagine it's faster than just going

var m = this.size
val it = coll.iterator()
while (m > n) it.next
while (it.hasNext) b += it.next()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would have the benefit of requiring only one traversal, indeed (excepted in the case of linear collections). I should give it a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it but didn’t notice a difference (in List):

takeright

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, what? List shouldn't rebuild on takeRight anyway--if it does, that needs to be fixed. Also, I didn't suggest the other method is faster, just more obviously correct.

Copy link
Contributor Author

@julienrf julienrf Jul 19, 2017

Choose a reason for hiding this comment

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

Indeed good point, that’s anyway something that we should fix… (#162 keeps track of that)

it.next()
}
while (it.hasNext) b += it.next()
b.result()
}

/** Takes longest prefix of elements that satisfy a predicate.
* $orderDependent
Expand Down Expand Up @@ -506,7 +517,17 @@ trait IterableOps[+A, +CC[X], +C] extends Any {
/** The rest of the collection without its `n` last elements. For
* linear, immutable collections this should avoid making a copy.
*/
def dropRight(n: Int): C = fromSpecificIterable(View.DropRight(coll, n))
def dropRight(n: Int): C = {
val b = newSpecificBuilder()
if (n >= 0) b.sizeHint(coll, delta = -n)
val lead = coll.iterator() drop n
val it = coll.iterator()
while (lead.hasNext) {
b += it.next
lead.next()
}
b.result()
}

/** Skips longest sequence of elements of this iterator which satisfy given
* predicate `p`, and returns an iterator of the remaining elements.
Expand Down
72 changes: 0 additions & 72 deletions src/main/scala/strawman/collection/Iterator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -379,44 +379,6 @@ trait Iterator[+A] extends IterableOnce[A] { self =>
else Iterator.empty.next()
}

/*
* Implemented by means of a buffer to keep track of the last n elements during iteration.
*/
def takeRight(n: Int): Iterator[A] = {
if (n <= 0) Iterator.empty
else {
// Return an iterator that iterates over the elements via a buffer
new Iterator[A]() {
private[this] var index = 0
private[this] var count = 0
// Use a lazy val for the buffer to make sure initialization is done only if needed and at most once
private[this] lazy val buffer = {
// Iterate over all elements while keeping track of the last n
var buf = ArrayBuffer[A]()
while (self.hasNext) {
if (index >= buf.length) buf += self.next()
else buf(index) = self.next()
index = if ((index + 1) >= n) 0 else index + 1
if (count < n) count += 1
}
// Adjust the starting index if needed
index = if (index >= buf.length) 0 else index
buf
}
def hasNext: Boolean = {
// Force initialization of buffer and return whether there are any elements left in the buffer
buffer != null && count > 0
}
def next(): A = {
val value = buffer(index)
index = if (index + 1 >= buffer.length) 0 else index + 1
count -= 1
value
}
}
}
}

/** Takes longest prefix of values produced by this iterator that satisfy a predicate.
*
* @param p The predicate used to test elements.
Expand Down Expand Up @@ -448,40 +410,6 @@ trait Iterator[+A] extends IterableOnce[A] { self =>
this
}

/*
* Implemented by means of a buffer to keep the last n elements from being returned during iteration.
*/
def dropRight(n: Int): Iterator[A] = {
if (n <= 0) self
else {
// Return an iterator that returns already buffered elements as it buffers new ones (using a buffer of most n elements)
new Iterator[A]() {
private[this] var index = 0
private[this] lazy val buffer = {
// Fill the buffer with the first n elements (or fewer if the n is greater than the iterator length)
val buf = ArrayBuffer[A]()
while (index < n && self.hasNext) {
buf += self.next()
index += 1
}
index = 0
buf
}

def hasNext: Boolean = {
// Force initialization of buffer and don't stop until the iterator is exhausted (without having returned the elements currently in the buffer since those are the ones to drop)
buffer != null && self.hasNext
}
def next(): A = {
val value = buffer(index)
buffer(index) = self.next()
index = if (index + 1 >= buffer.length) 0 else index + 1
value
}
}
}
}

/** Skips longest sequence of elements of this iterator which satisfy given
* predicate `p`, and returns an iterator of the remaining elements.
*
Expand Down
40 changes: 11 additions & 29 deletions src/main/scala/strawman/collection/View.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,6 @@ object View extends IterableFactory[View] {
if (underlying.knownSize >= 0) (underlying.knownSize - normN) max 0 else -1
}

/** A view that drops trailing elements of the underlying collection. */
case class DropRight[A](underlying: Iterable[A], n: Int) extends View[A] {
def iterator() = underlying.iterator().dropRight(n)
protected val normN = n max 0
override def knownSize =
if (underlying.knownSize >= 0) (underlying.knownSize - normN) max 0 else -1
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the DropRight view which was based on the dropRight method of Iterator. (same for TakeRight below)


case class DropWhile[A](underlying: Iterable[A], p: A => Boolean) extends View[A] {
def iterator() = underlying.iterator().dropWhile(p)
}
Expand All @@ -133,14 +125,6 @@ object View extends IterableFactory[View] {
if (underlying.knownSize >= 0) underlying.knownSize min normN else -1
}

/** A view that takes trailing elements of the underlying collection. */
case class TakeRight[A](underlying: Iterable[A], n: Int) extends View[A] {
def iterator() = underlying.iterator().takeRight(n)
protected val normN = n max 0
override def knownSize =
if (underlying.knownSize >= 0) underlying.knownSize min normN else -1
}

case class TakeWhile[A](underlying: Iterable[A], p: A => Boolean) extends View[A] {
def iterator(): Iterator[A] = underlying.iterator().takeWhile(p)
}
Expand Down Expand Up @@ -269,6 +253,8 @@ trait IndexedView[+A] extends View[A] with ArrayLike[A] { self =>
}
}

override def knownSize: Int = length
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s unrelated to the PR, actually, but I thought we could override knownSize in the case of IndexedView. We should do the same for IndexedSeq.


override def take(n: Int): IndexedView[A] = new IndexedView.Take(this, n)
override def takeRight(n: Int): IndexedView[A] = new IndexedView.TakeRight(this, n)
override def drop(n: Int): IndexedView[A] = new IndexedView.Drop(this, n)
Expand All @@ -286,11 +272,10 @@ object IndexedView {
def apply(i: Int) = underlying.apply(i)
}

class TakeRight[A](underlying: IndexedView[A], n: Int)
extends View.TakeRight(underlying, n) with IndexedView[A] {
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 TakeRight[A](underlying: IndexedView[A], n: Int) extends IndexedView[A] {
private[this] val delta = (underlying.length - (n max 0)) max 0
def length = underlying.length - delta
def apply(i: Int) = underlying.apply(i + delta)
}

class Drop[A](underlying: IndexedView[A], n: Int)
Expand All @@ -300,16 +285,13 @@ object IndexedView {
def apply(i: Int) = underlying.apply(i + normN)
}

class DropRight[A](underlying: IndexedView[A], n: Int)
extends View.DropRight(underlying, n) with IndexedView[A] {
override def iterator() = super.iterator()
def length = (underlying.length - normN) max 0
def apply(i: Int) = underlying.apply(i + normN)
class DropRight[A](underlying: IndexedView[A], n: Int) extends IndexedView[A] {
private[this] val len = (underlying.length - (n max 0)) max 0
def length = len
def apply(i: Int) = underlying.apply(i)
}

class Map[A, B](underlying: IndexedView[A], f: A => B)
extends View.Map(underlying, f) with IndexedView[B] {
override def iterator() = super.iterator()
class Map[A, B](underlying: IndexedView[A], f: A => B) extends IndexedView[B] {
def length = underlying.length
def apply(n: Int) = f(underlying.apply(n))
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/scala/strawman/collection/mutable/ArrayBuffer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ object ArrayBuffer extends IterableFactory[ArrayBuffer] {
}
else new ArrayBuffer[B] ++= coll

def newBuilder[A](): Builder[A, ArrayBuffer[A]] = new GrowableBuilder(empty[A])
def newBuilder[A](): Builder[A, ArrayBuffer[A]] =
new GrowableBuilder[A, ArrayBuffer[A]](empty) {
override def sizeHint(size: Int): Unit = elems.ensureSize(size)
}

def empty[A]: ArrayBuffer[A] = new ArrayBuffer[A]()
}
Expand Down
35 changes: 35 additions & 0 deletions src/main/scala/strawman/collection/mutable/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,41 @@ trait Builder[-A, +To] extends Growable[A] { self =>
*/
def sizeHint(size: Int): Unit = ()

/** Gives a hint that one expects the `result` of this builder
* to have the same size as the given collection, plus some delta. This will
* provide a hint only if the collection has a known size
* Some builder classes
* will optimize their representation based on the hint. However,
* builder implementations are still required to work correctly even if the hint is
* wrong, i.e. a different number of elements is added.
*
* @param coll the collection which serves as a hint for the result's size.
* @param delta a correction to add to the `coll.size` to produce the size hint.
*/
final def sizeHint(coll: strawman.collection.Iterable[_], delta: Int): Unit = {
if (coll.knownSize != -1) {
sizeHint(coll.knownSize + delta)
}
}

/** Gives a hint how many elements are expected to be added
* when the next `result` is called, together with an upper bound
* given by the size of some other collection. Some builder classes
* will optimize their representation based on the hint. However,
* builder implementations are still required to work correctly even if the hint is
* wrong, i.e. a different number of elements is added.
*
* @param size the hint how many elements will be added.
* @param boundingColl the bounding collection. If it is
* an IndexedSeqLike, then sizes larger
* than collection's size are reduced.
*/
final def sizeHintBounded(size: Int, boundingColl: strawman.collection.Iterable[_]): Unit = {
if (boundingColl.knownSize != -1) {
sizeHint(scala.math.min(boundingColl.knownSize, size))
}
}

/** A builder resulting from this builder my mapping the result using `f`. */
def mapResult[NewTo](f: To => NewTo) = new Builder[A, NewTo] {
def add(x: A): this.type = { self += x; this }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import scala.Unit
* @define Coll `GrowingBuilder`
* @define coll growing builder
*/
class GrowableBuilder[Elem, To <: Growable[Elem]](elems: To) extends Builder[Elem, To] {
class GrowableBuilder[Elem, To <: Growable[Elem]](protected val elems: To) extends Builder[Elem, To] {
def clear(): Unit = elems.clear()
def result(): To = elems
def add(elem: Elem): this.type = { elems += elem; this }
Expand Down