garydgregory commented on code in PR #509:
URL: 
https://github.com/apache/commons-collections/pull/509#discussion_r1693178853


##########
src/test/java/org/apache/commons/collections4/iterators/CartesianProductIteratorTest.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+
+/**
+ * Test class for {@link CartesianProductIterator}.
+ */
+public class CartesianProductIteratorTest extends 
AbstractIteratorTest<List<Character>> {
+
+    private List<Character> letters;
+    private List<Character> numbers;
+    private List<Character> symbols;
+
+    public CartesianProductIteratorTest() {
+        super(CartesianProductIteratorTest.class.getSimpleName());
+    }
+
+    @Override
+    public CartesianProductIterator<Character> makeEmptyIterator() {
+        return new CartesianProductIterator<>();
+    }
+
+    @Override
+    public CartesianProductIterator<Character> makeObject() {
+        return new CartesianProductIterator<>(letters, numbers, symbols);
+    }
+
+    @BeforeEach
+    public void setUp() {
+        letters = Arrays.asList('A', 'B', 'C');
+        numbers = Arrays.asList('1', '2', '3');
+        symbols = Arrays.asList('!', '?');
+    }
+
+    @Override
+    public boolean supportsRemove() {
+        return false;
+    }
+
+    @Test
+    public void testEmptyCollection() {
+        final CartesianProductIterator<Character> it = new 
CartesianProductIterator<>(letters, Collections.emptyList());
+        assertFalse(it.hasNext());
+        assertThrows(NoSuchElementException.class, () -> it.next());

Review Comment:
   Use a method reference for `next()`.



##########
src/test/java/org/apache/commons/collections4/iterators/CartesianProductIteratorTest.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+
+/**
+ * Test class for {@link CartesianProductIterator}.
+ */
+public class CartesianProductIteratorTest extends 
AbstractIteratorTest<List<Character>> {
+
+    private List<Character> letters;
+    private List<Character> numbers;
+    private List<Character> symbols;
+
+    public CartesianProductIteratorTest() {
+        super(CartesianProductIteratorTest.class.getSimpleName());
+    }
+
+    @Override
+    public CartesianProductIterator<Character> makeEmptyIterator() {
+        return new CartesianProductIterator<>();
+    }
+
+    @Override
+    public CartesianProductIterator<Character> makeObject() {
+        return new CartesianProductIterator<>(letters, numbers, symbols);
+    }
+
+    @BeforeEach
+    public void setUp() {
+        letters = Arrays.asList('A', 'B', 'C');
+        numbers = Arrays.asList('1', '2', '3');
+        symbols = Arrays.asList('!', '?');
+    }
+
+    @Override
+    public boolean supportsRemove() {
+        return false;
+    }
+
+    @Test
+    public void testEmptyCollection() {
+        final CartesianProductIterator<Character> it = new 
CartesianProductIterator<>(letters, Collections.emptyList());
+        assertFalse(it.hasNext());
+        assertThrows(NoSuchElementException.class, () -> it.next());
+    }
+
+    /**
+     * test checking that all the tuples are returned
+     */
+    @Test
+    public void testCartesianProductExhaustivity() {
+        final List<Character[]> resultsList = new ArrayList<>();
+

Review Comment:
   Whitespace is not needed.



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {
+        Objects.requireNonNull(iterables, "iterables");
+        this.iterables = Arrays.asList(iterables);
+        this.iterators = new ArrayList<>(iterables.length);
+        for (final Iterable<? extends E> iterable : iterables) {
+            Objects.requireNonNull(iterable, "iterable");
+            final Iterator<? extends E> iterator = iterable.iterator();
+            if (iterator.hasNext()) {
+                iterators.add(iterator);
+            } else {
+                iterators.clear();
+                break;
+            }
+        }
+    }
+
+    /**
+     * Indicates if there are more tuples to return.
+     *
+     * @return true if there are more tuples, otherwise false
+     */
+    @Override
+    public boolean hasNext() {
+        for (final Iterator<? extends E> iterator : iterators) {

Review Comment:
   You can use the less verbose `return 
iterators.stream().anyMatch(Iterator::hasNext);`



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {
+        Objects.requireNonNull(iterables, "iterables");
+        this.iterables = Arrays.asList(iterables);
+        this.iterators = new ArrayList<>(iterables.length);
+        for (final Iterable<? extends E> iterable : iterables) {
+            Objects.requireNonNull(iterable, "iterable");
+            final Iterator<? extends E> iterator = iterable.iterator();
+            if (iterator.hasNext()) {
+                iterators.add(iterator);
+            } else {
+                iterators.clear();
+                break;
+            }
+        }
+    }
+
+    /**
+     * Indicates if there are more tuples to return.

Review Comment:
   "Indicates" is not what you normally see in Javadocs. When in doubt, look at 
the specification in the interface:
   ```
   Returns {@code true} if the iteration has more elements.
   ```



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {

Review Comment:
   Since the code has null-guards, you can use `@SafeVarargs`.



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {
+        Objects.requireNonNull(iterables, "iterables");
+        this.iterables = Arrays.asList(iterables);
+        this.iterators = new ArrayList<>(iterables.length);
+        for (final Iterable<? extends E> iterable : iterables) {
+            Objects.requireNonNull(iterable, "iterable");
+            final Iterator<? extends E> iterator = iterable.iterator();
+            if (iterator.hasNext()) {
+                iterators.add(iterator);
+            } else {
+                iterators.clear();
+                break;
+            }
+        }
+    }
+
+    /**
+     * Indicates if there are more tuples to return.
+     *
+     * @return true if there are more tuples, otherwise false
+     */
+    @Override
+    public boolean hasNext() {
+        for (final Iterator<? extends E> iterator : iterators) {
+            if (iterator.hasNext()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Returns the next tuple of the input iterables.
+     *
+     * @return a list of the input iterables' elements
+     * @throws NoSuchElementException if there are no more tuples
+     */
+    @Override
+    public List<E> next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
+
+        if (previousTuple == null) {
+            previousTuple = new ArrayList<>(iterables.size());
+            for (final Iterator<? extends E> iterator : iterators) {
+                previousTuple.add(iterator.next());
+            }
+            return new ArrayList<>(previousTuple);
+        }
+
+        for (int i = iterators.size() - 1; i >= 0; i--) {
+            Iterator<? extends E> iterator = iterators.get(i);
+            if (iterator.hasNext()) {
+                previousTuple.set(i, iterator.next());
+                return new ArrayList<>(previousTuple);
+            }
+            iterator = iterables.get(i).iterator();
+            iterators.set(i, iterator);
+            previousTuple.set(i, iterator.next());
+        }
+        throw new IllegalStateException();
+    }
+
+    @Override
+    public void remove() {
+        throw new UnsupportedOperationException("remove() is not supported");

Review Comment:
   Keep the message language-neutral, like `java.util.Iterator.remove()`:
   ```java
   throw new UnsupportedOperationException("remove");
   ```
   



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.

Review Comment:
   "Standard" does not mean anything IMO. I would say "Constructs a new 
instance that ...".



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {
+        Objects.requireNonNull(iterables, "iterables");
+        this.iterables = Arrays.asList(iterables);
+        this.iterators = new ArrayList<>(iterables.length);
+        for (final Iterable<? extends E> iterable : iterables) {
+            Objects.requireNonNull(iterable, "iterable");
+            final Iterator<? extends E> iterator = iterable.iterator();
+            if (iterator.hasNext()) {
+                iterators.add(iterator);
+            } else {
+                iterators.clear();
+                break;
+            }
+        }
+    }
+
+    /**
+     * Indicates if there are more tuples to return.
+     *
+     * @return true if there are more tuples, otherwise false
+     */
+    @Override
+    public boolean hasNext() {
+        for (final Iterator<? extends E> iterator : iterators) {
+            if (iterator.hasNext()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Returns the next tuple of the input iterables.
+     *
+     * @return a list of the input iterables' elements
+     * @throws NoSuchElementException if there are no more tuples
+     */
+    @Override
+    public List<E> next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
+
+        if (previousTuple == null) {
+            previousTuple = new ArrayList<>(iterables.size());
+            for (final Iterator<? extends E> iterator : iterators) {
+                previousTuple.add(iterator.next());
+            }
+            return new ArrayList<>(previousTuple);
+        }
+
+        for (int i = iterators.size() - 1; i >= 0; i--) {
+            Iterator<? extends E> iterator = iterators.get(i);
+            if (iterator.hasNext()) {
+                previousTuple.set(i, iterator.next());
+                return new ArrayList<>(previousTuple);
+            }
+            iterator = iterables.get(i).iterator();
+            iterators.set(i, iterator);
+            previousTuple.set(i, iterator.next());
+        }
+        throw new IllegalStateException();

Review Comment:
   Describe the state in the exception message.



##########
src/main/java/org/apache/commons/collections4/iterators/CartesianProductIterator.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.commons.collections4.iterators;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+/**
+ * This iterator creates a Cartesian product of the input iterables,
+ * equivalent to nested for-loops.
+ * <p>
+ * The iterables provided to the constructor are used in reverse order, each
+ * until exhaustion before proceeding to the next element of the prior iterable
+ * and repeating. Consider the following example:
+ *
+ * <pre>{@code
+ * List<Character> iterable1 = Arrays.asList('A', 'B', 'C');
+ * List<Character> iterable2 = Arrays.asList('1', '2', '3');
+ * CartesianProductIterator<Character> it = new CartesianProductIterator<>(
+ *         iterable1,
+ *         iterable2);
+ * while (it.hasNext()) {
+ *     List<Character> tuple = it.next();
+ *     System.out.println(tuple.get(0) + ", " + tuple.get(1));
+ * }
+ * }</pre>
+ *
+ * The output will be:
+ *
+ * <pre>
+ * A, 1
+ * A, 2
+ * A, 3
+ * B, 1
+ * B, 2
+ * B, 3
+ * C, 1
+ * C, 2
+ * C, 3
+ * </pre>
+ * <p>
+ * The {@code remove()} operation is not supported, and will throw an
+ * {@code UnsupportedOperationException}.
+ *
+ * @param <E> the type of the objects being permuted
+ * @since 4.5.0
+ */
+public class CartesianProductIterator<E> implements Iterator<List<E>> {
+
+    /**
+     * The iterables to create the Cartesian product from.
+     */
+    private final List<Iterable<? extends E>> iterables;
+
+    /**
+     * The iterators to generate the Cartesian product tuple from.
+     */
+    private final List<Iterator<? extends E>> iterators;
+
+    /**
+     * The previous generated tuple of elements.
+     */
+    private List<E> previousTuple;
+
+    /**
+     * Standard constructor for this class.
+     *
+     * @param iterables the iterables to create the Cartesian product from
+     * @throws NullPointerException if any of the iterables is null
+     */
+    public CartesianProductIterator(final Iterable<? extends E>... iterables) {
+        Objects.requireNonNull(iterables, "iterables");
+        this.iterables = Arrays.asList(iterables);
+        this.iterators = new ArrayList<>(iterables.length);
+        for (final Iterable<? extends E> iterable : iterables) {
+            Objects.requireNonNull(iterable, "iterable");
+            final Iterator<? extends E> iterator = iterable.iterator();
+            if (iterator.hasNext()) {

Review Comment:
   You don't need both blocks nested if you say:
   ```java
               if (!iterator.hasNext()) {
                   iterators.clear();
                   break;
               }
               iterators.add(iterator);
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to