Skip to content

Commit

Permalink
Make Iterators.peekingIterator() reuse AbstractIterator.peek().
Browse files Browse the repository at this point in the history
Currently, `Iterators.peekingIterator()` will treat `AbstractIterator`
just like any other iterator, and will call `next()` to get the element,
which it then stashes. This is wasteful, since `AbstractIterator` already
provides a `peek()` method that is designed to be compatible with the
`PeekingIterator` interface, so use it instead.

`AbstractIterator` subclasses `UnmodifiableIterator`, so we can do
likewise with the `PeekingIterator` forwarder. The resulting forwarder
is much more lightweight than the default `Iterators.peekingIterator()`
implementation.
  • Loading branch information
cky committed Apr 7, 2020
1 parent 31999ae commit 5bbe4d4
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,18 @@ private void assertNextThrows(Iterator<?> iterator) {
} catch (ThrowsAtEndException expected) {
}
}

private static class UniqueObjectIterator extends AbstractIterator<Object> {
@Override
protected Object computeNext() {
return new Object();
}
}

public void testPeekingAbstractIteratorDoesntAdvancePrematurely() {
Iterator<Object> iterator = new UniqueObjectIterator();
PeekingIterator<Object> peeking1 = Iterators.peekingIterator(iterator);
PeekingIterator<Object> peeking2 = Iterators.peekingIterator(iterator);
assertSame(peeking1.peek(), peeking2.peek());
}
}
40 changes: 40 additions & 0 deletions guava/src/com/google/common/collect/Iterators.java
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,30 @@ public E peek() {
}
}

private static class AbstractIteratorPeekingImpl<E> extends UnmodifiableIterator<E>
implements PeekingIterator<E> {
private final AbstractIterator<? extends E> iterator;

public AbstractIteratorPeekingImpl(AbstractIterator<? extends E> iterator) {
this.iterator = checkNotNull(iterator);
}

@Override
public boolean hasNext() {
return iterator.hasNext();
}

@Override
public E next() {
return iterator.next();
}

@Override
public E peek() {
return iterator.peek();
}
}

/**
* Returns a {@code PeekingIterator} backed by the given iterator.
*
Expand Down Expand Up @@ -1198,6 +1222,22 @@ public static <T> PeekingIterator<T> peekingIterator(Iterator<? extends T> itera
PeekingImpl<T> peeking = (PeekingImpl<T>) iterator;
return peeking;
}
if (iterator instanceof AbstractIteratorPeekingImpl) {
// Safe to cast <? extends T> to <T> because AbstractIteratorPeekingImpl
// only uses T covariantly (and cannot be subclassed to add non-covariant
// uses).
@SuppressWarnings("unchecked")
AbstractIteratorPeekingImpl<T> peeking = (AbstractIteratorPeekingImpl<T>) iterator;
return peeking;
}
return peekingImpl(iterator);
}

/* Captures the iterator's element type so the downcast does not cause warnings. */
private static <T, E extends T> PeekingIterator<T> peekingImpl(Iterator<E> iterator) {
if (iterator instanceof AbstractIterator) {
return new AbstractIteratorPeekingImpl<T>((AbstractIterator<E>) iterator);
}
return new PeekingImpl<T>(iterator);
}

Expand Down

0 comments on commit 5bbe4d4

Please sign in to comment.