From 5bbe4d4e42f9605c45cbca7066222fef5b5eee77 Mon Sep 17 00:00:00 2001 From: Chris Jester-Young Date: Mon, 6 Apr 2020 20:19:10 -0400 Subject: [PATCH] Make `Iterators.peekingIterator()` reuse `AbstractIterator.peek()`. 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. --- .../common/collect/PeekingIteratorTest.java | 14 +++++++ .../com/google/common/collect/Iterators.java | 40 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/guava-tests/test/com/google/common/collect/PeekingIteratorTest.java b/guava-tests/test/com/google/common/collect/PeekingIteratorTest.java index 0f79985bcf39..5d6a80f5a0e4 100644 --- a/guava-tests/test/com/google/common/collect/PeekingIteratorTest.java +++ b/guava-tests/test/com/google/common/collect/PeekingIteratorTest.java @@ -257,4 +257,18 @@ private void assertNextThrows(Iterator iterator) { } catch (ThrowsAtEndException expected) { } } + + private static class UniqueObjectIterator extends AbstractIterator { + @Override + protected Object computeNext() { + return new Object(); + } + } + + public void testPeekingAbstractIteratorDoesntAdvancePrematurely() { + Iterator iterator = new UniqueObjectIterator(); + PeekingIterator peeking1 = Iterators.peekingIterator(iterator); + PeekingIterator peeking2 = Iterators.peekingIterator(iterator); + assertSame(peeking1.peek(), peeking2.peek()); + } } diff --git a/guava/src/com/google/common/collect/Iterators.java b/guava/src/com/google/common/collect/Iterators.java index f655c279de53..4c3bcf5a28b9 100644 --- a/guava/src/com/google/common/collect/Iterators.java +++ b/guava/src/com/google/common/collect/Iterators.java @@ -1154,6 +1154,30 @@ public E peek() { } } + private static class AbstractIteratorPeekingImpl extends UnmodifiableIterator + implements PeekingIterator { + private final AbstractIterator iterator; + + public AbstractIteratorPeekingImpl(AbstractIterator 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. * @@ -1198,6 +1222,22 @@ public static PeekingIterator peekingIterator(Iterator itera PeekingImpl peeking = (PeekingImpl) iterator; return peeking; } + if (iterator instanceof AbstractIteratorPeekingImpl) { + // Safe to cast to because AbstractIteratorPeekingImpl + // only uses T covariantly (and cannot be subclassed to add non-covariant + // uses). + @SuppressWarnings("unchecked") + AbstractIteratorPeekingImpl peeking = (AbstractIteratorPeekingImpl) iterator; + return peeking; + } + return peekingImpl(iterator); + } + + /* Captures the iterator's element type so the downcast does not cause warnings. */ + private static PeekingIterator peekingImpl(Iterator iterator) { + if (iterator instanceof AbstractIterator) { + return new AbstractIteratorPeekingImpl((AbstractIterator) iterator); + } return new PeekingImpl(iterator); }