Skip to content

Commit fea75db

Browse files
committed
Make Iterators.peekingIterator() reuse AbstractIterator.peek().
Currently, `Iterators.peekingIterator()` will treat `AbstractIterator` just like any other iterator, and will call `next` to 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.
1 parent 31999ae commit fea75db

File tree

2 files changed

+54
-0
lines changed

2 files changed

+54
-0
lines changed

guava-tests/test/com/google/common/collect/PeekingIteratorTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,18 @@ private void assertNextThrows(Iterator<?> iterator) {
257257
} catch (ThrowsAtEndException expected) {
258258
}
259259
}
260+
261+
private static class UniqueObjectIterator extends AbstractIterator<Object> {
262+
@Override
263+
protected Object computeNext() {
264+
return new Object();
265+
}
266+
}
267+
268+
public void testPeekingAbstractIteratorDoesntAdvancePrematurely() {
269+
Iterator<Object> iterator = new UniqueObjectIterator();
270+
PeekingIterator<Object> peeking = Iterators.peekingIterator(iterator);
271+
Object object = peeking.peek();
272+
assertSame(object, iterator.next());
273+
}
260274
}

guava/src/com/google/common/collect/Iterators.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,30 @@ public E peek() {
11541154
}
11551155
}
11561156

1157+
private static class AbstractIteratorPeekingImpl<E> extends UnmodifiableIterator<E>
1158+
implements PeekingIterator<E> {
1159+
private final AbstractIterator<? extends E> iterator;
1160+
1161+
public AbstractIteratorPeekingImpl(AbstractIterator<? extends E> iterator) {
1162+
this.iterator = checkNotNull(iterator);
1163+
}
1164+
1165+
@Override
1166+
public boolean hasNext() {
1167+
return iterator.hasNext();
1168+
}
1169+
1170+
@Override
1171+
public E next() {
1172+
return iterator.next();
1173+
}
1174+
1175+
@Override
1176+
public E peek() {
1177+
return iterator.peek();
1178+
}
1179+
}
1180+
11571181
/**
11581182
* Returns a {@code PeekingIterator} backed by the given iterator.
11591183
*
@@ -1198,6 +1222,22 @@ public static <T> PeekingIterator<T> peekingIterator(Iterator<? extends T> itera
11981222
PeekingImpl<T> peeking = (PeekingImpl<T>) iterator;
11991223
return peeking;
12001224
}
1225+
if (iterator instanceof AbstractIteratorPeekingImpl) {
1226+
// Safe to cast <? extends T> to <T> because AbstractIteratorPeekingImpl
1227+
// only uses T covariantly (and cannot be subclassed to add non-covariant
1228+
// uses).
1229+
@SuppressWarnings("unchecked")
1230+
AbstractIteratorPeekingImpl<T> peeking = (AbstractIteratorPeekingImpl<T>) iterator;
1231+
return peeking;
1232+
}
1233+
return peekingImpl(iterator);
1234+
}
1235+
1236+
/* Captures the iterator's element type so the downcast does not cause warnings. */
1237+
private static <T, E extends T> PeekingIterator<T> peekingImpl(Iterator<E> iterator) {
1238+
if (iterator instanceof AbstractIterator) {
1239+
return new AbstractIteratorPeekingImpl<T>((AbstractIterator<E>) iterator);
1240+
}
12011241
return new PeekingImpl<T>(iterator);
12021242
}
12031243

0 commit comments

Comments
 (0)