You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/06/01 19:42:32 UTC

[GitHub] [commons-collections] garydgregory commented on a change in pull request #238: [COLLECTIONS-795] Add a new Iterator to allowing zipping over two iterators of different types

garydgregory commented on a change in pull request #238:
URL: https://github.com/apache/commons-collections/pull/238#discussion_r643425608



##########
File path: src/main/java/org/apache/commons/collections4/iterators/PairedIterator.java
##########
@@ -0,0 +1,171 @@
+/*
+ * 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 java.util.Objects.requireNonNull;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import org.apache.commons.collections4.iterators.PairedIterator.PairedItem;
+
+/**
+ * Provides a iteration over the elements contained in a pair of Iterators.
+ *
+ * <p>
+ * Given two {@link Iterator} instances {@code A} and {@code B}, the {@link #next} method on this
+ * iterator provide a Pair of {@code A.next()} and {@code B.next()} until one of the iterators is
+ * exhausted.
+ * </p>
+ * Example usage:
+ * <pre>{@code
+ *   List<Integer> studentIds = ...
+ *   List<String> studentNames = ...
+ *
+ *   PairedIterator<PairedItem<Integer, String>> pairedIterator =
+ *     PairedIterator.ofIterables(studentIds, studentNames);
+ *
+ *   while (pairedIterator.hasNext()) {
+ *     PairedItem<Integer, String> item = zippedIterator.next();
+ *     ...
+ *   }
+ * }</pre>
+ *
+ * @param <L> the left elements' type
+ * @param <R> the right elements' type
+ */
+public class PairedIterator<L, R> implements Iterator<PairedItem<L, R>> {
+
+    /**
+     * The left {@link Iterator}s to evaluate.
+     */
+    private final Iterator<L> leftIterator;
+
+    /**
+     * The right {@link Iterator}s to evaluate.
+     */
+    private final Iterator<R> rightIterator;
+
+    // Constructor
+    // ----------------------------------------------------------------------
+
+    /**
+     * Constructs a new {@code ZipPairIterator} that will provide  iteration over the two given
+     * iterators.
+     *
+     * @param leftIterator  the iterator for the left side element.
+     * @param rightIterator the iterator for the right side element.
+     * @throws NullPointerException if either iterator is null
+     */
+    public PairedIterator(Iterator<L> leftIterator, Iterator<R> rightIterator) {
+        this.leftIterator = requireNonNull(leftIterator);
+        this.rightIterator = requireNonNull(rightIterator);
+    }
+
+    /**
+     * Convenience static factory to construct the ZipPairIterator
+     *
+     * @param leftIterator  the iterator for the left side element.
+     * @param rightIterator the iterator for the right side element.
+     * @return the iterator to iterate over the provided iterators.
+     * @throws NullPointerException if either iterator is null
+     */
+    public static <L, R> PairedIterator<L, R> of(Iterator<L> leftIterator, Iterator<R> rightIterator) {
+        return new PairedIterator<>(leftIterator, rightIterator);
+    }
+
+    /**
+     * Convenience static factory to construct the ZipPairIterator from any {@link Iterable} sources.
+     *
+     * @param leftIterable  the iterable for the left side element.
+     * @param rightIterable the iterable for the right side element.
+     * @return the iterator to iterate over the iterators derived from the provided iterables.
+     * @throws NullPointerException if either iterables is null
+     */
+    public static <L, R> PairedIterator<L, R> ofIterables(Iterable<L> leftIterable, Iterable<R> rightIterable) {
+        return of(requireNonNull(leftIterable).iterator(), requireNonNull(rightIterable).iterator());
+    }
+
+    // Iterator Methods
+    // -------------------------------------------------------------------
+
+    /**
+     * Returns {@code true} if both the child iterators have remaining elements.
+     *
+     * @return true if both the child iterators have remaining elements
+     */
+    @Override
+    public boolean hasNext() {
+        return leftIterator.hasNext() && rightIterator.hasNext();
+    }
+
+    /**
+     * Returns the next elements from both the child iterators.
+     *
+     * @return the next elements from both the iterators.
+     * @throws NoSuchElementException if any one child iterator is exhausted.
+     */
+    @Override
+    public PairedItem<L, R> next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
+
+        return PairedItem.of(leftIterator.next(), rightIterator.next());
+    }
+
+    /**
+     * An immutable tuple class to represent elements from both the iterators.
+     *
+     * @param <L> the left elements' type
+     * @param <R> the right elements' type
+     */

Review comment:
       We need to think about this because Common Lang already defines different kinds of Pair classes...

##########
File path: src/test/java/org/apache/commons/collections4/iterators/PairedIteratorTest.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.Iterator;
+import java.util.Random;
+import java.util.UUID;
+import org.apache.commons.collections4.IteratorUtils;
+import org.apache.commons.collections4.iterators.PairedIterator.PairedItem;
+
+
+/** Unit test suite for {@link ZippingIterator}. */
+public final class PairedIteratorTest
+    extends AbstractIteratorTest<PairedItem<String, Integer>> {
+
+    //------------------------------------------------------------ Conventional
+
+    public PairedIteratorTest(String testName) {
+        super(testName);
+    }
+
+    //------------------------------------------------------------ Lifecycle
+
+    private ArrayList<String> smallStringsList = null;
+    private ArrayList<String> largeStringsList = null;
+    private ArrayList<Integer> smallIntsList = null;
+    private ArrayList<Integer> largeIntsList = null;
+
+    // Unequal sized lists
+    private static final int SMALL_LIST_SIZE = 20;

Review comment:
       If you expect the sizes to be different, you should assert that fact somewhere. 

##########
File path: src/main/java/org/apache/commons/collections4/IteratorUtils.java
##########
@@ -907,6 +908,20 @@ public static NodeListIterator nodeListIterator(final Node node) {
         return new ZippingIterator<>(iterators);
     }
 
+    /**
+     * Returns an iterator that provides the elements contained in a pair of Iterators.
+     *
+     * @param <L> the left elements' type
+     * @param <R> the right elements' type
+     * @param left the iterator for the left side elements
+     * @param right the iterator for the right side elements
+     * @return an iterator, to iterate over the decorated iterators together until one is exhausted
+     * @throws NullPointerException if any iterator is null

Review comment:
       All new public and protected methods should have a Javadoc since 4.5 tag.

##########
File path: src/test/java/org/apache/commons/collections4/PairedIterableTest.java
##########
@@ -0,0 +1,84 @@
+/*
+ * 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;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Random;
+import java.util.UUID;
+import java.util.stream.Collectors;
+import org.apache.commons.collections4.iterators.PairedIterator.PairedItem;
+import org.junit.Before;
+import org.junit.Test;
+
+public final class PairedIterableTest {
+    private ArrayList<String> smallStringsList = null;

Review comment:
       Don't override default vslues with default values.

##########
File path: src/test/java/org/apache/commons/collections4/iterators/PairedIteratorTest.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.Iterator;
+import java.util.Random;
+import java.util.UUID;
+import org.apache.commons.collections4.IteratorUtils;
+import org.apache.commons.collections4.iterators.PairedIterator.PairedItem;
+
+
+/** Unit test suite for {@link ZippingIterator}. */
+public final class PairedIteratorTest
+    extends AbstractIteratorTest<PairedItem<String, Integer>> {
+
+    //------------------------------------------------------------ Conventional

Review comment:
       No need for these inline comments, they document nothing.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org