CompositeIterator decouple from CompositeIterable

redisson/src/main/java/org/redisson/misc/CompositeIterable.java

@@ -19,14 +19,11 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
-public class CompositeIterable<T> implements Iterable<T>, Iterator<T> {
+public class CompositeIterable<T> implements Iterable<T> {
 
     private List<Iterable<T>> iterablesList;
     private Iterable<T>[] iterables;
 
-    private Iterator<Iterator<T>> listIterator;
-    private Iterator<T> currentIterator;
-
     public CompositeIterable(List<Iterable<T>> iterables) {
         this.iterablesList = iterables;
     }
@@ -52,35 +49,7 @@ public class CompositeIterable<T> implements Iterable<T>, Iterator<T> {
                 iterators.add(iterable.iterator());
             }
         }
-        listIterator = iterators.iterator();
-        currentIterator = null;
-        return this;
+        Iterator<Iterator<T>>  listIterator = iterators.iterator();
+        return new CompositeIterator<T>(listIterator);
     }
-
-    @Override
-    public boolean hasNext() {
-        if (currentIterator == null || !currentIterator.hasNext()) {
-            while (listIterator.hasNext()) {
-                Iterator<T> iterator = listIterator.next();
-                if (iterator.hasNext()) {
-                    currentIterator = iterator;
-                    return true;
-                }
-            }
-            return false;
-        }
-        return currentIterator.hasNext();
-    }
-
-    @Override
-    public T next() {
-        hasNext();
-        return currentIterator.next();
-    }
-
-    @Override
-    public void remove() {
-        currentIterator.remove();
-    }
-
 }

redisson/src/main/java/org/redisson/misc/CompositeIterator.java

@@ -0,0 +1,66 @@
+/**
+ * Copyright 2018 Nikita Koksharov
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.redisson.misc;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+/**
+ * @author Pepe Lu
+ */
+public class CompositeIterator<T> implements Iterator<T> {
+
+	private Iterator<Iterator<T>> listIterator;
+	private Iterator<T> currentIterator;
+
+	public CompositeIterator(Iterator<Iterator<T>> iterators) {
+		listIterator = iterators;
+	}
+
+	@Override
+	public boolean hasNext() {
+		if (currentIterator == null || !currentIterator.hasNext()) {
+			while (listIterator.hasNext()) {
+				Iterator<T> iterator = listIterator.next();
+				currentIterator = iterator;
+				if (iterator.hasNext()) {
+					return true;
+				}
+			}
+			return false;
+		}
+
+		return currentIterator.hasNext();
+	}
+
+	@Override
+	public T next() {
+		if (!hasNext()) {
+			throw new NoSuchElementException();
+		}
+
+		return currentIterator.next();
+	}
+
+	@Override
+	public void remove() {
+		if (currentIterator == null) {
+			throw new IllegalStateException("next() has not yet been called");
+		}
+
+		currentIterator.remove();
+	}
+}

redisson/src/test/java/org/redisson/misc/CompositeIteratorTest.java

@@ -0,0 +1,132 @@
+package org.redisson.misc;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertFalse;
+
+import java.util.*;
+
+import org.junit.Test;
+
+/**
+ * @author Pepe Lu
+ */
+public class CompositeIteratorTest {
+
+	@Test
+	public void testHasNextWithEmpty() {
+		List<Integer> emptyList = new ArrayList<Integer>();
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				emptyList);
+		assertFalse(compositeIterable.iterator().hasNext());
+	}
+
+	@Test(expected = NoSuchElementException.class)
+	public void testNextWithEmpty() {
+		List<Integer> emptyList = new ArrayList<Integer>();
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				emptyList);
+		compositeIterable.iterator().next();
+	}
+
+	@Test
+	public void testNextWithOne() {
+		List<Integer> list = Arrays.asList(1, 2);
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				list);
+		Iterator<Integer> iterator = compositeIterable.iterator();
+		assertThat(iterator.next()).isEqualTo(1);
+		assertThat(iterator.next()).isEqualTo(2);
+		assertFalse(iterator.hasNext());
+	}
+
+	@Test
+	public void testNextWithTwo() {
+		List<Integer> list1 = Arrays.asList(1, 2);
+		List<Integer> list2 = Arrays.asList(3, 4);
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				list1, list2);
+		Iterator<Integer> iterator = compositeIterable.iterator();
+		assertThat(iterator.next()).isEqualTo(1);
+		assertThat(iterator.next()).isEqualTo(2);
+		assertThat(iterator.next()).isEqualTo(3);
+		assertThat(iterator.next()).isEqualTo(4);
+		assertFalse(iterator.hasNext());
+	}
+
+	@Test(expected = IllegalStateException.class)
+	public void testRemoveWithEmpty() {
+		List<Integer> emptyList = new ArrayList<Integer>();
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				emptyList);
+		compositeIterable.iterator().remove();
+	}
+
+	@Test
+	public void testRemoveWithOne() {
+		/**
+		 * use ArrayList instead of Arrays.asList() because Arrays.asList() is using
+		 * {@link Arrays.ArrayList} which is using {@link AbstractList.Itr} as its
+		 * iterator. And this iterator does not implement remove()
+		 */
+		List<Integer> list = new ArrayList<Integer>();
+		list.add(1);
+		list.add(2);
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				list);
+		Iterator<Integer> iterator = compositeIterable.iterator();
+		int count = list.size();
+		while (iterator.hasNext()) {
+			iterator.next();
+			iterator.remove();
+			count--;
+		}
+
+		assertThat(count).isEqualTo(0);
+		assertThat(list.size()).isEqualTo(0);
+	}
+
+	@Test
+	public void testRemoveWithTwo() {
+		List<Integer> list1 = new ArrayList<Integer>();
+		list1.add(1);
+		list1.add(2);
+		List<Integer> list2 = new ArrayList<Integer>();
+		list2.add(3);
+		list2.add(4);
+		CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+				list1, list2);
+		Iterator<Integer> iterator = compositeIterable.iterator();
+		int count = list1.size() + list2.size();
+		while (iterator.hasNext()) {
+			iterator.next();
+			iterator.remove();
+			count--;
+		}
+
+		assertThat(count).isEqualTo(0);
+        assertThat(list1.size()).isEqualTo(0);
+        assertThat(list2.size()).isEqualTo(0);
+	}
+
+	@Test
+    public void testPartialIterationWithTwo(){
+        List<Integer> list1 = new ArrayList<Integer>();
+        list1.add(1);
+        list1.add(2);
+        List<Integer> list2 = new ArrayList<Integer>();
+        list2.add(3);
+        list2.add(4);
+        CompositeIterable<Integer> compositeIterable = new CompositeIterable<Integer>(
+                list1, list2);
+        Iterator<Integer> iterator = compositeIterable.iterator();
+        iterator.next();
+        iterator.remove();
+        iterator.next();
+        iterator.next();
+        iterator.remove();
+        assertThat(list1.size()).isEqualTo(1);
+        assertThat(list2.size()).isEqualTo(1);
+        assertThat(list1.get(0)).isEqualTo(2);
+        assertThat(list2.get(0)).isEqualTo(4);
+    }
+}