Simplifies GenericIterator and adds tests for State implementations.

src/main/java/org/javimmutable/collections/iterators/GenericIterator.java

@@ -152,30 +152,56 @@ default T value()
         /**
          * Try to move forward to the next position.  Returns either a valid state (if next position exists)
          * or null (if there is no next position).  The returned State might be this State object or a new
-         * State, or null.  The returned State might have a value or it might be empty.
+         * State, or null.  The returned State might have a value or it might be empty and in need of another
+         * call to {@link #advance()} to reach a value.
          */
         @Nullable
         State<T> advance();
+
+        /**
+         * Chains calls to {@link #advance()} until wither a value is reached or null is returned.
+         * If already at a value just returns the state.
+         */
+        static <T> State<T> advanceUntilHasValue(State<T> state)
+        {
+            while (state != null && !state.hasValue()) {
+                state = state.advance();
+            }
+            return state;
+        }
+
+        /**
+         * Advances past current value (if any) and then chains calls to {@link #advance()} until wither a value
+         * is reached or null is returned.
+         */
+        static <T> State<T> advanceToNextValue(State<T> state)
+        {
+            if (state != null) {
+                return advanceUntilHasValue(state.advance());
+            } else {
+                return null;
+            }
+        }
     }
 
     @Override
     public synchronized boolean hasNext()
     {
-        advanceStateToNextPositionIfNecessary();
+        initializeStateIfNecessary();
         return stateHasValue();
     }
 
     @Override
     public synchronized T next()
     {
-        advanceStateToNextPositionIfNecessary();
+        initializeStateIfNecessary();
         if (!stateHasValue()) {
             throw new NoSuchElementException();
         }
         final T answer = state.value();
         offset += 1;
         if (offset < limit) {
-            state = state.advance();
+            state = State.advanceToNextValue(state);
         } else {
             state = null;
         }
@@ -199,22 +225,18 @@ public synchronized SplitIterator<T> splitIterator()
     }
 
     /**
-     * Ensures that state is on a valid position if possible.  Gets a starting state if we are
-     * starting a new iteration otherwise it calls advance if necessary to move to the first
-     * available position.
+     * Gets an initial state if we are starting a new iteration and advances it to the first
+     * value if possible.  Does nothing if we are already initialized.
      */
-    private void advanceStateToNextPositionIfNecessary()
+    private void initializeStateIfNecessary()
     {
         if (uninitialized) {
             // create a starting state on first pass
+            assert state == null;
             state = root.iterateOverRange(null, offset, limit);
+            state = State.advanceUntilHasValue(state);
             uninitialized = false;
         }
-        // Advance until we reach a state with a value or run out of states to try.
-        // We need the loop because it's possible for a state to be empty but be followed by a non-empty one.
-        while (state != null && !state.hasValue()) {
-            state = state.advance();
-        }
     }
 
     private boolean stateHasValue()
@@ -226,9 +248,16 @@ private boolean stateHasValue()
      * Returns a State for iterating a single value.
      */
     public static <T> State<T> singleValueState(State<T> parent,
-                                                T value)
+                                                T value,
+                                                int offset,
+                                                int limit)
     {
-        return new SingleValueState<>(parent, value);
+        assert offset >= 0 && offset <= limit && limit <= 1;
+        if (offset == limit) {
+            return parent;
+        } else {
+            return new SingleValueState<>(parent, value);
+        }
     }
 
     /**
@@ -243,12 +272,7 @@ public State<T> iterateOverRange(@Nullable State<T> parent,
                                              int offset,
                                              int limit)
             {
-                assert offset >= 0 && offset <= limit && limit <= 1;
-                if (offset == limit) {
-                    return parent;
-                } else {
-                    return new SingleValueState<>(parent, value);
-                }
+                return singleValueState(parent, value, offset, limit);
             }
 
             @Override
@@ -268,7 +292,11 @@ public static <T> State<T> multiValueState(@Nullable State<T> parent,
                                                int limit)
     {
         assert offset >= 0 && offset <= limit && limit <= values.size();
-        return new MultiValueState<>(parent, values, offset, limit);
+        if (offset == limit) {
+            return parent;
+        } else {
+            return new MultiValueState<>(parent, values, offset, limit);
+        }
     }
 
     /**
@@ -334,34 +362,29 @@ private static class SingleValueState<T>
     {
         private final State<T> parent;
         private final T value;
-        private boolean available;
 
         private SingleValueState(@Nullable State<T> parent,
                                  T value)
         {
             this.parent = parent;
             this.value = value;
-            available = true;
         }
 
         @Override
         public boolean hasValue()
         {
-            return available;
+            return true;
         }
 
         @Override
         public T value()
         {
-            assert available;
-            available = false;
             return value;
         }
 
         @Override
         public State<T> advance()
         {
-            assert !available;
             return parent;
         }
     }
@@ -379,6 +402,7 @@ private MultiValueState(@Nullable GenericIterator.State<T> parent,
                                 int offset,
                                 int limit)
         {
+            assert offset >= 0 && offset < limit;
             this.parent = parent;
             this.values = values;
             this.offset = offset;
@@ -424,6 +448,7 @@ private MultiIterableState(@Nullable State<T> parent,
                                    int offset,
                                    int limit)
         {
+            assert offset >= 0 && offset < limit;
             this.parent = parent;
             this.collections = collections;
             this.limit = limit;

src/main/java/org/javimmutable/collections/list/OneValueNode.java

@@ -255,7 +255,7 @@ public GenericIterator.State<T> iterateOverRange(@Nullable GenericIterator.State
                                                      int offset,
                                                      int limit)
     {
-        return GenericIterator.singleValueState(parent, value);
+        return GenericIterator.singleValueState(parent, value, offset, limit);
     }
 
     @Override

src/main/java/org/javimmutable/collections/tree/LeafNode.java

@@ -257,7 +257,7 @@ public GenericIterator.State<IMapEntry<K, V>> iterateOverRange(@Nullable Generic
                                                                    int offset,
                                                                    int limit)
     {
-        return GenericIterator.singleValueState(parent, asEntry());
+        return GenericIterator.singleValueState(parent, asEntry(), offset, limit);
     }
 
     @Override

src/test/java/org/javimmutable/collections/iterators/GenericIteratorTest.java

@@ -35,6 +35,7 @@
 
 package org.javimmutable.collections.iterators;
 
+import static java.util.Arrays.asList;
 import junit.framework.TestCase;
 import org.javimmutable.collections.IStreamable;
 import org.javimmutable.collections.Indexed;
@@ -43,17 +44,17 @@
 import org.javimmutable.collections.common.StreamConstants;
 import org.javimmutable.collections.indexed.IndexedArray;
 import org.javimmutable.collections.indexed.IndexedHelper;
+import org.javimmutable.collections.indexed.IndexedList;
+import static org.javimmutable.collections.iterators.GenericIterator.MIN_SIZE_FOR_SPLIT;
+import static org.javimmutable.collections.iterators.StandardIteratorTests.verifyOrderedIterable;
+import static org.javimmutable.collections.iterators.StandardIteratorTests.verifyOrderedSplit;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
-import static java.util.Arrays.asList;
-import static org.javimmutable.collections.iterators.GenericIterator.MIN_SIZE_FOR_SPLIT;
-import static org.javimmutable.collections.iterators.StandardIteratorTests.*;
-
 public class GenericIteratorTest
     extends TestCase
 {
@@ -94,6 +95,56 @@ public void testStandard()
         verifyOrderedSplit(true, lr(1, len / 2), lr(len / 2 + 1, len), deep.iterator());
     }
 
+    public void testSingleValueState()
+    {
+        verifyStateOperations(GenericIterator.singleValueState(null, 1, 0, 1), List.of(1));
+        verifyStateOperations(GenericIterator.singleValueState(null, 1, 1, 1), List.of());
+    }
+
+    public void testMultiValueState()
+    {
+        verifyStateOperations(GenericIterator.multiValueState(null, IndexedHelper.indexed(1, 2, 3), 0, 3), List.of(1, 2, 3));
+        verifyStateOperations(GenericIterator.multiValueState(null, IndexedHelper.indexed(1, 2, 3), 1, 3), List.of(2, 3));
+        verifyStateOperations(GenericIterator.multiValueState(null, IndexedHelper.indexed(1, 2, 3), 2, 3), List.of(3));
+        verifyStateOperations(GenericIterator.multiValueState(null, IndexedHelper.indexed(1, 2, 3), 3, 3), List.of());
+    }
+
+    public void testMultiIterableState()
+    {
+        for (int numLists = 1; numLists <= 3; ++numLists) {
+            List<List<List<Integer>>> combos = ListCombinations.combosOfListsOfLength(numLists, 0, 3);
+            ListCombinations.renumberCombos(combos);
+            for (List<List<Integer>> combo : combos) {
+                List<SimpleIterable<Integer>> iterables = new ArrayList<>();
+                for (List<Integer> integers : combo) {
+                    iterables.add(new SimpleIterable<>(IndexedList.retained(integers)));
+                }
+                List<Integer> expected = ListCombinations.valuesFrom(combo);
+                Indexed<GenericIterator.Iterable<Integer>> values = IndexedList.retained(iterables);
+                GenericIterator.State<Integer> state = GenericIterator.multiIterableState(null, values, 0, expected.size());
+                verifyStateOperations(state, expected);
+            }
+        }
+    }
+
+    private void verifyStateOperations(GenericIterator.State<Integer> state,
+                                       List<Integer> expected)
+    {
+        state = GenericIterator.State.advanceUntilHasValue(state);
+        if (expected.isEmpty()) {
+            assertNull(state);
+        } else {
+            for (Integer x : expected) {
+                assertNotNull(state);
+                assertEquals(true, state.hasValue());
+                assertEquals(x, state.value());
+                assertEquals(x, state.value());
+                state = GenericIterator.State.advanceToNextValue(state);
+            }
+        }
+        assertEquals(null, state);
+    }
+
     private int limit(int multiple)
     {
         return multiple * MIN_SIZE_FOR_SPLIT;
@@ -165,6 +216,32 @@ private static List<Integer> lr(int first,
         return list;
     }
 
+    private static class SimpleIterable<T>
+        implements GenericIterator.Iterable<T>
+    {
+        private final Indexed<T> values;
+
+        private SimpleIterable(Indexed<T> values)
+        {
+            this.values = values;
+        }
+
+        @Nullable
+        @Override
+        public GenericIterator.State<T> iterateOverRange(@Nullable GenericIterator.State<T> parent,
+                                                         int offset,
+                                                         int limit)
+        {
+            return GenericIterator.multiValueState(parent, values, offset, limit);
+        }
+
+        @Override
+        public int iterableSize()
+        {
+            return values.size();
+        }
+    }
+
     private static abstract class Node
         implements GenericIterator.Iterable<Integer>,
                    IStreamable<Integer>
@@ -206,7 +283,7 @@ public GenericIterator.State<Integer> iterateOverRange(@Nullable GenericIterator
                                                                int offset,
                                                                int limit)
         {
-            return GenericIterator.singleValueState(parent, value);
+            return GenericIterator.singleValueState(parent, value, offset, limit);
         }
     }