From 656207eb3f169ae9d184685380b04e87aa69f88f Mon Sep 17 00:00:00 2001 From: "Gregory.Shrago" Date: Wed, 16 Nov 2016 01:36:25 +0300 Subject: [PATCH] JBIterator: do not advance cursor iterator on each hasNext() --- .../util/containers/TreeTraverserTest.java | 31 ++++- .../intellij/util/containers/JBIterable.java | 2 +- .../intellij/util/containers/JBIterator.java | 117 ++++++++++++------ 3 files changed, 108 insertions(+), 42 deletions(-) diff --git a/platform/platform-tests/testSrc/com/intellij/util/containers/TreeTraverserTest.java b/platform/platform-tests/testSrc/com/intellij/util/containers/TreeTraverserTest.java index fe004304c77f..5d84ccc49f8d 100644 --- a/platform/platform-tests/testSrc/com/intellij/util/containers/TreeTraverserTest.java +++ b/platform/platform-tests/testSrc/com/intellij/util/containers/TreeTraverserTest.java @@ -172,14 +172,39 @@ public class TreeTraverserTest extends TestCase { assertEquals(new Integer(1), it.current()); } - public void testIteratorContractsCursor() { + public void testCursorIterableContract() { List list = ContainerUtil.newArrayList(); - for (JBIterator it : JBIterator.cursor(JBIterator.from(JBIterable.of(1, 2).iterator()))) { + JBIterable orig = JBIterable.generate(1, INCREMENT).take(5); + for (JBIterator it : JBIterator.cursor(JBIterator.from(orig.iterator()))) { it.current(); it.hasNext(); list.add(it.current()); } - assertEquals(Arrays.asList(1, 2), list); + assertEquals(orig.toList(), list); + } + + public void testCursorIteratorContract() { + JBIterable orig = JBIterable.generate(1, INCREMENT).take(5); + JBIterator> it = JBIterator.from(JBIterator.cursor( + JBIterator.from(orig.iterator())).iterator()); + List list = ContainerUtil.newArrayList(); + while (it.advance()) { + it.hasNext(); + list.add(it.current().current()); + } + assertEquals(orig.toList(), list); + } + + public void testCursorTransform() { + JBIterable orig = JBIterable.generate(1, INCREMENT).take(5); + + List expected = ContainerUtil.newArrayList(1, 2, 3, 4, 5); + List expectedOdd = ContainerUtil.newArrayList(1, 3, 5); + assertEquals(expected, JBIterator.cursor(JBIterator.from(orig.iterator())).transform(o -> o.current()).toList()); + assertEquals(expected.size(), JBIterator.cursor(JBIterator.from(orig.iterator())).last().current().intValue()); + assertEquals(expectedOdd, JBIterator.cursor(JBIterator.from(orig.iterator())).transform(o -> o.current()).filter(IS_ODD).toList()); + assertEquals(expectedOdd, JBIterator.cursor(JBIterator.from(orig.iterator())).filter(o -> IS_ODD.value(o.current())).transform(o -> o.current()).toList()); + assertEquals(expected.subList(0, 4), JBIterator.cursor(JBIterator.from(orig.iterator())).filter(o -> o.hasNext()).transform(o -> o.current()).toList()); } public void testIteratorContractsSkipAndStop() { diff --git a/platform/util/src/com/intellij/util/containers/JBIterable.java b/platform/util/src/com/intellij/util/containers/JBIterable.java index d772bb48f305..88c532a90440 100644 --- a/platform/util/src/com/intellij/util/containers/JBIterable.java +++ b/platform/util/src/com/intellij/util/containers/JBIterable.java @@ -210,7 +210,7 @@ public abstract class JBIterable implements Iterable { @NotNull @Override public String toString() { - return myIterable == this ? super.toString() : String.valueOf(myIterable); + return myIterable == this ? JBIterable.class.getSimpleName() : String.valueOf(myIterable); } /** diff --git a/platform/util/src/com/intellij/util/containers/JBIterator.java b/platform/util/src/com/intellij/util/containers/JBIterator.java index 73356e3d136e..eff41eead1ba 100644 --- a/platform/util/src/com/intellij/util/containers/JBIterator.java +++ b/platform/util/src/com/intellij/util/containers/JBIterator.java @@ -46,16 +46,13 @@ import java.util.NoSuchElementException; * * @author gregsh * - * @noinspection unchecked, AssignmentToForLoopParameter + * @noinspection unchecked, TypeParameterHidesVisibleType, AssignmentToForLoopParameter */ public abstract class JBIterator implements Iterator { - private static final Object NONE = new String("#none"); - private static final Object STOP = new String("#stop"); - private static final Object SKIP = new String("#skip"); @NotNull public static > JBIterable cursor(@NotNull E iterator) { - return JBIterable.generate(iterator, Functions.identity()).takeWhile(ADVANCE); + return JBIterable.generate(iterator, Functions.id()).intercept(CURSOR_NEXT); } @NotNull @@ -73,10 +70,11 @@ public abstract class JBIterator implements Iterator { }; } - private Object myCurrent = NONE; - private Object myNext = NONE; + private enum Do {INIT, STOP, SKIP} + private Object myCurrent = Do.INIT; + private Object myNext = Do.INIT; - private Op myFirstOp = new Op(null); + private Op myFirstOp = new NextOp(); private Op myLastOp = myFirstOp; /** @@ -94,7 +92,7 @@ public abstract class JBIterator implements Iterator { */ @Nullable protected final E stop() { - myNext = STOP; + myNext = Do.STOP; return null; } @@ -103,14 +101,14 @@ public abstract class JBIterator implements Iterator { */ @Nullable protected final E skip() { - myNext = SKIP; + myNext = Do.SKIP; return null; } @Override public final boolean hasNext() { peekNext(); - return myNext != STOP; + return myNext != Do.STOP; } @Override @@ -123,11 +121,14 @@ public abstract class JBIterator implements Iterator { * Proceeds to the next element if any and returns true; otherwise false. */ public final boolean advance() { - myCurrent = NONE; + myCurrent = Do.INIT; peekNext(); - if (myNext == STOP) return false; + if (myNext == Do.STOP) return false; myCurrent = myNext; - myNext = NONE; + myNext = Do.INIT; + if (myFirstOp instanceof JBIterator.CursorOp) { + ((CursorOp)myFirstOp).advance(myCurrent); + } currentChanged(); return true; } @@ -136,20 +137,22 @@ public abstract class JBIterator implements Iterator { * Returns the current element if any; otherwise throws exception. */ public final E current() { - if (myCurrent == NONE) throw new NoSuchElementException(); + if (myCurrent == Do.INIT) { + throw new NoSuchElementException(); + } return (E)myCurrent; } private void peekNext() { - if (myNext != NONE) return; - Object o = NONE; + if (myNext != Do.INIT) return; + Object o = Do.INIT; for (Op op = myFirstOp; op != null; op = op == null ? myFirstOp : op.nextOp) { - o = op.impl == null ? nextImpl() : op.apply(o); - if (myNext == SKIP) { - o = myNext = NONE; + o = op.apply(op.impl == null ? nextImpl() : o); + if (myNext == Do.STOP) return; + if (myNext == Do.SKIP) { + o = myNext = Do.INIT; op = null; } - if (myNext == STOP) return; } myNext = o; } @@ -167,7 +170,7 @@ public abstract class JBIterator implements Iterator { @NotNull public final JBIterator take(int count) { // add first so that the underlying iterator stay on 'count' position - return addOp(myLastOp.impl != null, new WhileOp(new CountDown(count))); + return addOp(!(myLastOp instanceof NextOp), new WhileOp(new CountDown(count))); } @NotNull @@ -187,7 +190,10 @@ public abstract class JBIterator implements Iterator { @NotNull private T addOp(boolean last, @NotNull Op op) { - if (last) { + if (op.impl == null) { + myFirstOp = myLastOp = op; + } + else if (last) { myLastOp.nextOp = op; myLastOp = myLastOp.nextOp; } @@ -210,8 +216,8 @@ public abstract class JBIterator implements Iterator { @Override public String toString() { - JBIterable ops = operationsImpl(); - return "{cur=" + myCurrent + "; next=" + myNext + (ops.isEmpty() ? "" : "; ops[" + ops.size() + "]=" + ops) + "}"; + List ops = operationsImpl().toList(); + return "{cur=" + myCurrent + "; next=" + myNext + (ops.size() < 2 ? "" : "; ops=" + ops) + "}"; } @NotNull @@ -226,7 +232,7 @@ public abstract class JBIterator implements Iterator { @NotNull private JBIterable operationsImpl() { - return JBIterable.generate(myFirstOp.nextOp, new Function() { + return JBIterable.generate(myFirstOp, new Function() { @Override public Op fun(Op op) { return op.nextOp; @@ -234,15 +240,20 @@ public abstract class JBIterator implements Iterator { }); } + @NotNull static String toShortString(@NotNull Object o) { - String fqn = o.getClass().getName(); - return StringUtil.replace(o.toString(), fqn, StringUtil.getShortName(fqn, '.')); + String name = o.getClass().getName(); + int idx = name.lastIndexOf('$'); + if (idx > 0 && idx < name.length() && StringUtil.isJavaIdentifierStart(name.charAt(idx + 1))) { + return name.substring(idx + 1); + } + return name.substring(name.lastIndexOf('.') + 1); } - private static final Condition> ADVANCE = new Condition>() { + private static final Function.Mono CURSOR_NEXT = new Function.Mono>() { @Override - public boolean value(JBIterator it) { - return it.advance(); + public JBIterator fun(JBIterator iterator) { + return iterator.addOp(false, iterator.new CursorOp()); } }; @@ -260,7 +271,7 @@ public abstract class JBIterator implements Iterator { @Override public String toString() { - return impl == null ? "" : toShortString(impl); + return toShortString(impl == null ? this : impl); } } @@ -283,7 +294,7 @@ public abstract class JBIterator implements Iterator { } @Override - public Object apply(Object o) { + Object apply(Object o) { return impl.fun((E)o); } } @@ -294,7 +305,7 @@ public abstract class JBIterator implements Iterator { } @Override - public Object apply(Object o) { + Object apply(Object o) { return impl.value((E)o) ? o : skip(); } } @@ -305,24 +316,54 @@ public abstract class JBIterator implements Iterator { super(condition); } @Override - public Object apply(Object o) { + Object apply(Object o) { return impl.value((E)o) ? o : stop(); } } private class SkipOp extends Op> { - boolean active; + boolean active = true; SkipOp(Condition condition) { super(condition); - active = true; } @Override - public Object apply(Object o) { + Object apply(Object o) { if (active && impl.value((E)o)) return skip(); active = false; return o; } } + + private static class NextOp extends Op { + NextOp() { + super(null); + } + + @Override + Object apply(Object o) { + return o; + } + } + + private class CursorOp extends Op { + boolean advanced; + + CursorOp() { + super(null); + } + + @Override + Object apply(Object o) { + JBIterator it = (JBIterator)o; + return ((advanced = nextOp != null) ? it.advance() : it.hasNext()) ? it : stop(); + } + + void advance(Object o) { + if (advanced || !(o instanceof JBIterator)) return; + ((JBIterator)o).advance(); + advanced = true; + } + } }