From 04b05e725d09fe077c12a67be5639f57b5612e9f Mon Sep 17 00:00:00 2001 From: Igor Rudenko Date: Mon, 3 Aug 2026 15:03:00 +0300 Subject: [PATCH 1/5] [COLLECTIONS-897] Add LexicographicPermutationIterator Add an Iterator> that generates the permutations of a collection in lexicographical order, complementing PermutationIterator, which uses the Steinhaus-Johnson-Trotter ordering. Elements are ordered by their natural ordering, or by a Comparator supplied to the two-argument constructor, which also allows permuting elements that do not implement Comparable. Each call to next() advances by the standard next-permutation step: locate the pivot, swap it with its successor, then reverse the descending tail. Equal elements are not distinguished, so an input with duplicates yields fewer than n! permutations. An empty collection yields exactly one empty list, as 0! = 1. remove() is unsupported. Comparator dispatch follows the java.util.TreeMap pattern of testing the comparator field for null on each comparison; benchmarking showed no measurable difference against normalizing null to Comparator.naturalOrder() in the constructor. Tests extend AbstractIteratorTest to cover the Iterator contract, and add cases for lexicographical exhaustivity, duplicate handling, custom and reverse comparators, non-Comparable elements, stream traversal, exhaustion, and equals/hashCode. --- .../LexicographicPermutationIterator.java | 198 ++++++++++ .../LexicographicPermutationIteratorTest.java | 343 ++++++++++++++++++ 2 files changed, 541 insertions(+) create mode 100644 src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java create mode 100644 src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java diff --git a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java new file mode 100644 index 0000000000..3389db68b2 --- /dev/null +++ b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java @@ -0,0 +1,198 @@ +/* + * 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 + * + * https://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.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Objects; + +/** + * This iterator creates permutations of an input collection, using the + * lexicographical order. + *

+ * The iterator might return fewer than n! permutations of the input collection, + * because duplicated permutations are skipped: equal elements are not + * distinguished from one another. + * The {@code remove()} operation is not supported, and will throw an + * {@code UnsupportedOperationException}. + *

+ *

+ * NOTE: in case an empty collection is provided, the iterator will + * return exactly one empty list as result, as 0! = 1. + *

+ * + * @param the type of the objects being permuted + * @see PermutationIterator + * @since 4.6.0 + */ +public class LexicographicPermutationIterator implements Iterator> { + + /** + * The comparator used to define order of generation, + * or null if it uses the natural ordering. + */ + private final Comparator comparator; + + /** + * Next permutation to return. When a permutation is requested + * this instance is provided and the next one is computed. + */ + private List nextPermutation; + + /** + * Standard constructor for this class, using the natural ordering of the elements. + * + * @param collection The collection to generate permutations for + * @throws NullPointerException if collection is null + */ + public LexicographicPermutationIterator(final Collection collection) { + this(collection, null); + } + + /** + * Constructs an instance using the given comparator to order the elements. + * + * @param collection The collection to generate permutations for + * @param comparator The comparator used to define the order of generation, + * or null to use the natural ordering of the elements + * @throws NullPointerException if collection is null + */ + public LexicographicPermutationIterator(final Collection collection, final Comparator comparator) { + Objects.requireNonNull(collection, "collection"); + nextPermutation = new ArrayList<>(collection); + this.comparator = comparator; + } + + /** + * Indicates if there are more permutation available. + * + * @return true if there are more permutations, otherwise false + */ + @Override + public boolean hasNext() { + return nextPermutation != null; + } + + /** + * Returns the next permutation of the input collection. + * + * @return A list of the permutator's elements representing a permutation + * @throws NoSuchElementException if there are no more permutations + */ + @Override + public List next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + + final int size = nextPermutation.size(); + List nextP = null; + + // find the pivot: the rightmost element that is smaller than its successor. + // if there is none the current permutation is the last one in lexicographical order + int i = size - 2; + while (i >= 0 && compareElements(nextPermutation.get(i), nextPermutation.get(i + 1)) >= 0) { + --i; + } + + if (i >= 0) { + // find the rightmost element greater than the pivot; the tail is descending, + // so this is the pivot's successor in the remaining elements + int j = size - 1; + while (j >= i && compareElements(nextPermutation.get(i), nextPermutation.get(j)) >= 0) { + --j; + } + + // swap the pivot with its successor, then reverse the descending tail + // into ascending order to obtain the smallest larger permutation + nextP = new ArrayList<>(nextPermutation); + Collections.swap(nextP, i, j); + final List subList = nextP.subList(i + 1, nextP.size()); + Collections.reverse(subList); + } + + final List result = nextPermutation; + nextPermutation = nextP; + return result; + } + + /** + * Always throws {@link UnsupportedOperationException}. + * + * @throws UnsupportedOperationException Always thrown. + */ + @Override + public void remove() { + throw new UnsupportedOperationException("remove() is not supported"); + } + + /** + * Compares this iterator to another for equality. Two iterators are equal when + * they use equal comparators and are positioned at an equal next permutation. + * + * @param o The object to compare to this instance + * @return true if the given object is an equal iterator, otherwise false + */ + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + + if (o == null || getClass() != o.getClass()) { + return false; + } + + final LexicographicPermutationIterator that = (LexicographicPermutationIterator) o; + return Objects.equals(comparator, that.comparator) && Objects.equals(nextPermutation, that.nextPermutation); + } + + /** + * Returns a hash code consistent with {@link #equals(Object)}. Note that the + * hash code changes as the iterator advances. + * + * @return A hash code for this instance + */ + @Override + public int hashCode() { + return Objects.hash(comparator, nextPermutation); + } + + /** + * Compares two elements using the comparator, or their natural ordering if no + * comparator was supplied. + * + * @param e1 The first element to compare + * @param e2 The second element to compare + * @return a negative integer, zero, or a positive integer as the first element + * is less than, equal to, or greater than the second + * @throws ClassCastException if no comparator was supplied and the elements are + * not mutually {@link Comparable} + */ + @SuppressWarnings("unchecked") + private int compareElements(final E e1, final E e2) { + return comparator == null + ? ((Comparable) e1).compareTo(e2) + : comparator.compare(e1, e2); + } + +} diff --git a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java new file mode 100644 index 0000000000..c20dd8a13b --- /dev/null +++ b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java @@ -0,0 +1,343 @@ +/* + * 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 + * + * https://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.Collections.emptyList; +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 static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Test class for LexicographicPermutationIterator. + */ +class LexicographicPermutationIteratorTest extends AbstractIteratorTest> { + + /** + * A comparator that orders nothing, identified only by an id, used to check that + * equal comparators make equal iterators. + * + * @param the type of the objects compared + */ + private static final class CustomComparator implements Comparator { + + private final int id; + + CustomComparator(final int id) { + this.id = id; + } + + @Override + public int compare(final T o1, final T o2) { + return 0; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + + if (o == null || getClass() != o.getClass()) { + return false; + } + + final CustomComparator cmp = (CustomComparator) o; + return id == cmp.id; + } + + @Override + public int hashCode() { + return id; + } + } + + /** + * A value holder that deliberately does not implement {@link Comparable}, used to + * check that a supplied comparator is honored. + * + * @param the type of the wrapped value + */ + private static final class NonComparableObject { + + private final T value; + + NonComparableObject(final T value) { + this.value = value; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + + if (o == null || getClass() != o.getClass()) { + return false; + } + + final NonComparableObject that = (NonComparableObject) o; + return Objects.equals(value, that.value); + } + + T getValue() { + return value; + } + + @Override + public int hashCode() { + return Objects.hash(value); + } + } + + @SuppressWarnings("boxing") // OK in test code + protected Character[] testArray = { 'A', 'B', 'C' }; + + protected List testList; + + @Override + public LexicographicPermutationIterator makeEmptyIterator() { + return new LexicographicPermutationIterator<>(new ArrayList<>()); + } + + @Override + public LexicographicPermutationIterator makeObject() { + return new LexicographicPermutationIterator<>(testList); + } + + @BeforeEach + public void setUp() { + testList = new ArrayList<>(); + testList.addAll(Arrays.asList(testArray)); + } + + @Override + public boolean supportsEmptyIterator() { + return false; + } + + @Override + public boolean supportsRemove() { + return false; + } + + @Test + void testCustomComparator() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('C', 'B', 'A'), + Comparator.reverseOrder()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'B', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'C', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'C'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'C', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B', 'C'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + + @Test + void testCustomComparatorWithNonComparableObjects() { + final Iterator>> permutationIterator = + new LexicographicPermutationIterator<>(Arrays.asList( + new NonComparableObject<>('A'), + new NonComparableObject<>('B')), Comparator.comparing(NonComparableObject::getValue)); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList( + new NonComparableObject<>('A'), + new NonComparableObject<>('B')), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList( + new NonComparableObject<>('B'), + new NonComparableObject<>('A')), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + + @Test + void testDuplicatedPermutationsAreSkipped() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('A', 'A', 'B', 'B')); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'A', 'B', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B', 'A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B', 'B', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'B', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'B', 'A', 'A'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + + @Test + void testEmptyCollection() { + final Iterator> permutationIterator = makeEmptyIterator(); + + // there is one permutation for an empty set: 0! = 1 + assertTrue(permutationIterator.hasNext()); + assertTrue(permutationIterator.next().isEmpty()); + + assertFalse(permutationIterator.hasNext()); + } + + @Test + void testEqualsForEqualCollections() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList()); + + assertEquals(one, another); + } + + @Test + void testEqualsForEqualCollectionsAndComparators() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + + assertEquals(one, another); + } + + @Test + void testHashCodeForEqualCollections() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList()); + + assertEquals(one.hashCode(), another.hashCode()); + } + + @Test + void testHashCodeForEqualCollectionsAndComparators() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + + assertEquals(one.hashCode(), another.hashCode()); + } + + @Test + void testNonComparableElementsThrow() { + final Iterator>> permutationIterator = new LexicographicPermutationIterator<>( + Arrays.asList( + new NonComparableObject<>('A'), + new NonComparableObject<>('B'))); + + assertTrue(permutationIterator.hasNext()); + assertThrows(ClassCastException.class, permutationIterator::next); + } + + @Test + void testPermutationException() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A'), permutationIterator.next()); + + // asking for another permutation should throw an exception + assertFalse(permutationIterator.hasNext()); + assertThrows(NoSuchElementException.class, permutationIterator::next); + } + + /** + * test checking that all the permutations are returned in lexicographical order + */ + @Test + void testPermutationExhaustivity() { + final Iterator> permutationIterator = makeObject(); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B', 'C'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'C', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'C'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'C', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'B', 'A'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + + @Test + void testRemoveThrows() { + final Iterator> permutationIterator = makeObject(); + + assertTrue(permutationIterator.hasNext()); + assertThrows(UnsupportedOperationException.class, permutationIterator::remove); + } + + @Test + void testStreamOfPermutations() { + final Iterable> iterable = this::makeObject; + + final List> allPermutations = StreamSupport.stream(iterable.spliterator(), false) + .collect(Collectors.toList()); + + assertEquals(Arrays.asList( + Arrays.asList('A', 'B', 'C'), + Arrays.asList('A', 'C', 'B'), + Arrays.asList('B', 'A', 'C'), + Arrays.asList('B', 'C', 'A'), + Arrays.asList('C', 'A', 'B'), + Arrays.asList('C', 'B', 'A')), allPermutations); + } + +} From fb5995493234fd1e218a344e2032684e6ba2536c Mon Sep 17 00:00:00 2001 From: Igor Rudenko Date: Wed, 12 Aug 2026 18:27:44 +0300 Subject: [PATCH 2/5] Documentation improvements --- .../LexicographicPermutationIterator.java | 38 +++++++++++++- .../LexicographicPermutationIteratorTest.java | 51 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java index 3389db68b2..1c4f1f495c 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java @@ -29,9 +29,27 @@ * This iterator creates permutations of an input collection, using the * lexicographical order. *

+ * Iteration starts at the arrangement in which the elements are given, and each + * call to {@code next()} advances to the smallest arrangement greater than the + * current one. Iteration therefore ends at the largest arrangement, and the ones + * preceding the given arrangement are never returned: only a collection already + * sorted according to the ordering in use yields the complete set of + * permutations. Callers wanting the complete set must sort the collection + * beforehand, just as callers of + * {@link java.util.Collections#binarySearch(java.util.List, Object) binarySearch} + * must. Callers wanting to enumerate one part of the set, to resume from a + * previously reached arrangement or to split the work, may start anywhere. + *

+ *

+ * The starting arrangement is the iteration order of the given collection, so + * collections whose iteration order is unspecified, such as {@link java.util.HashSet}, + * make poor input: which permutations are returned is then unspecified too. + *

+ *

* The iterator might return fewer than n! permutations of the input collection, - * because duplicated permutations are skipped: equal elements are not - * distinguished from one another. + * either because the collection was not sorted according to the ordering in use, + * as described above, or because duplicated permutations are skipped: equal + * elements are not distinguished from one another. * The {@code remove()} operation is not supported, and will throw an * {@code UnsupportedOperationException}. *

@@ -39,6 +57,11 @@ * NOTE: in case an empty collection is provided, the iterator will * return exactly one empty list as result, as 0! = 1. *

+ *

+ * NOTE: {@link PermutationIterator} differs on both counts. It returns exactly n! + * permutations whatever the order of the input collection. The two iterators are + * therefore not interchangeable. + *

* * @param the type of the objects being permuted * @see PermutationIterator @@ -60,6 +83,10 @@ public class LexicographicPermutationIterator implements Iterator> { /** * Standard constructor for this class, using the natural ordering of the elements. + *

+ * Iteration starts at the arrangement in which the collection iterates its + * elements; sort the collection first to obtain the complete set of permutations. + *

* * @param collection The collection to generate permutations for * @throws NullPointerException if collection is null @@ -70,6 +97,11 @@ public LexicographicPermutationIterator(final Collection collection /** * Constructs an instance using the given comparator to order the elements. + *

+ * Iteration starts at the arrangement in which the collection iterates its + * elements; sort the collection with the same comparator first to obtain the + * complete set of permutations. + *

* * @param collection The collection to generate permutations for * @param comparator The comparator used to define the order of generation, @@ -97,6 +129,8 @@ public boolean hasNext() { * * @return A list of the permutator's elements representing a permutation * @throws NoSuchElementException if there are no more permutations + * @throws ClassCastException if no comparator was supplied and the elements are + * not mutually {@link Comparable} */ @Override public List next() { diff --git a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java index c20dd8a13b..c0ca12c357 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java +++ b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java @@ -340,4 +340,55 @@ void testStreamOfPermutations() { Arrays.asList('C', 'B', 'A')), allPermutations); } + /** + * test checking that iteration starts at the given arrangement rather than at the + * smallest one, so that a collection which is not sorted yields only the + * permutations that follow it. Sorting the input is the caller's responsibility, + * as it is for binarySearch. + */ + @Test + void testUnsortedCollectionStartsAtGivenArrangement() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('B', 'A', 'C')); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'C'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'C', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'A', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('C', 'B', 'A'), permutationIterator.next()); + + // the two permutations starting with 'A' precede the given arrangement + // and are therefore never returned + assertFalse(permutationIterator.hasNext()); + } + + /** + * test checking that the starting arrangement is honoured for a supplied comparator + * too, the permutations preceding it under that comparator being left out. + */ + @Test + void testUnsortedCollectionStartsAtGivenArrangementWithComparator() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('B', 'C', 'A'), + Comparator.reverseOrder()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'C', 'A'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('B', 'A', 'C'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'C', 'B'), permutationIterator.next()); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'B', 'C'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + } From 12278fa4d56e2ab5c6c7900e007c64abf591c940 Mon Sep 17 00:00:00 2001 From: Igor Rudenko Date: Wed, 12 Aug 2026 18:51:33 +0300 Subject: [PATCH 3/5] bump-up to the next release version 4.7.0 --- .../iterators/LexicographicPermutationIterator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java index 1c4f1f495c..f4a0089cd0 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java @@ -65,7 +65,7 @@ * * @param the type of the objects being permuted * @see PermutationIterator - * @since 4.6.0 + * @since 4.7.0 */ public class LexicographicPermutationIterator implements Iterator> { From 2a8bbeb91982e314b58698a903cf8022d44be30d Mon Sep 17 00:00:00 2001 From: Igor Rudenko Date: Mon, 17 Aug 2026 19:54:09 +0300 Subject: [PATCH 4/5] Improvements according to reviewer's comments What changed: - Javadoc polished - Missing tests added - `next()` now returns a defensive copy, covered by new tests --- .../LexicographicPermutationIterator.java | 81 ++++--- .../LexicographicPermutationIteratorTest.java | 227 ++++++++++++++++++ 2 files changed, 277 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java index f4a0089cd0..aad17a3794 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java @@ -76,8 +76,8 @@ public class LexicographicPermutationIterator implements Iterator> { private final Comparator comparator; /** - * Next permutation to return. When a permutation is requested - * this instance is provided and the next one is computed. + * Next permutation to return. When a permutation is requested a copy of this + * instance is provided and the next one is computed. */ private List nextPermutation; @@ -115,7 +115,7 @@ public LexicographicPermutationIterator(final Collection collection } /** - * Indicates if there are more permutation available. + * Indicates if there are more permutations available. * * @return true if there are more permutations, otherwise false */ @@ -126,6 +126,11 @@ public boolean hasNext() { /** * Returns the next permutation of the input collection. + *

+ * The returned list is a mutable copy taken before the iterator advances, never the + * iterator's own state, and it belongs to the caller. It may therefore be modified in + * any way and at any time without affecting the remaining permutations. + *

* * @return A list of the permutator's elements representing a permutation * @throws NoSuchElementException if there are no more permutations @@ -138,34 +143,8 @@ public List next() { throw new NoSuchElementException(); } - final int size = nextPermutation.size(); - List nextP = null; - - // find the pivot: the rightmost element that is smaller than its successor. - // if there is none the current permutation is the last one in lexicographical order - int i = size - 2; - while (i >= 0 && compareElements(nextPermutation.get(i), nextPermutation.get(i + 1)) >= 0) { - --i; - } - - if (i >= 0) { - // find the rightmost element greater than the pivot; the tail is descending, - // so this is the pivot's successor in the remaining elements - int j = size - 1; - while (j >= i && compareElements(nextPermutation.get(i), nextPermutation.get(j)) >= 0) { - --j; - } - - // swap the pivot with its successor, then reverse the descending tail - // into ascending order to obtain the smallest larger permutation - nextP = new ArrayList<>(nextPermutation); - Collections.swap(nextP, i, j); - final List subList = nextP.subList(i + 1, nextP.size()); - Collections.reverse(subList); - } - - final List result = nextPermutation; - nextPermutation = nextP; + final List result = new ArrayList<>(nextPermutation); + nextPermutation = smallestGreaterThan(nextPermutation); return result; } @@ -229,4 +208,44 @@ private int compareElements(final E e1, final E e2) { : comparator.compare(e1, e2); } + /** + * Returns the smallest arrangement of the given elements greater than the given one, + * as a new list. The permutation passed in is only read, never rearranged. + * + * @param permutation The arrangement to advance from + * @return A new list holding the next arrangement in lexicographical order, or null + * if the given one is already the largest + * @throws ClassCastException if no comparator was supplied and the elements are + * not mutually {@link Comparable} + */ + private List smallestGreaterThan(final List permutation) { + final int size = permutation.size(); + + // find the pivot: the rightmost element that is smaller than its successor. + // if there is none the given permutation is the last one in lexicographical order + int i = size - 2; + while (i >= 0 && compareElements(permutation.get(i), permutation.get(i + 1)) >= 0) { + --i; + } + + if (i < 0) { + return null; + } + + // find the rightmost element greater than the pivot; the tail is descending, + // so this is the pivot's successor in the remaining elements + int j = size - 1; + while (j >= i && compareElements(permutation.get(i), permutation.get(j)) >= 0) { + --j; + } + + // swap the pivot with its successor, then reverse the descending tail + // into ascending order to obtain the smallest larger permutation + final List nextP = new ArrayList<>(permutation); + Collections.swap(nextP, i, j); + final List subList = nextP.subList(i + 1, nextP.size()); + Collections.reverse(subList); + return nextP; + } + } diff --git a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java index c0ca12c357..38065909a8 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java +++ b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java @@ -19,6 +19,8 @@ import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -122,6 +124,17 @@ public int hashCode() { protected List testList; + /** + * Advances the given iterator until it holds no further permutation. + * + * @param iterator the iterator to exhaust + */ + private static void exhaust(final Iterator iterator) { + while (iterator.hasNext()) { + iterator.next(); + } + } + @Override public LexicographicPermutationIterator makeEmptyIterator() { return new LexicographicPermutationIterator<>(new ArrayList<>()); @@ -148,6 +161,22 @@ public boolean supportsRemove() { return false; } + /** + * test checking that a collection whose elements are all equal yields a single + * permutation, equal elements not being distinguished from one another: the n! + * arrangements are all duplicates of each other and collapse into one. + */ + @Test + void testAllEqualElementsYieldSinglePermutation() { + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('A', 'A', 'A')); + + assertTrue(permutationIterator.hasNext()); + assertEquals(Arrays.asList('A', 'A', 'A'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + assertThrows(NoSuchElementException.class, permutationIterator::next); + } + @Test void testCustomComparator() { final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('C', 'B', 'A'), @@ -230,6 +259,37 @@ void testEmptyCollection() { assertFalse(permutationIterator.hasNext()); } + @Test + void testEqualsForDifferentComparators() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(7)); + final Iterator> natural = new LexicographicPermutationIterator<>(emptyList()); + + assertNotEquals(one, another); + assertNotEquals(one, natural); + } + + @Test + void testEqualsForDifferentPositions() { + final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + + assertEquals(one, another); + + // advancing one of them leaves them at different permutations + one.next(); + assertNotEquals(one, another); + + // advancing the other brings them back to the same permutation + another.next(); + assertEquals(one, another); + + // an exhausted iterator differs from one still holding a permutation + one.next(); + assertFalse(one.hasNext()); + assertNotEquals(one, another); + } + @Test void testEqualsForEqualCollections() { final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); @@ -246,6 +306,97 @@ void testEqualsForEqualCollectionsAndComparators() { assertEquals(one, another); } + /** + * test checking that the comparator keeps being compared once both iterators are + * exhausted, so that two iterators which will both emit nothing are still unequal. + * The position is spent, but it is not the only part of the equality contract. + */ + @Test + void testEqualsForExhaustedIteratorsWithDifferentComparators() { + final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B'), new CustomComparator<>(42)); + final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B'), new CustomComparator<>(7)); + + exhaust(one); + exhaust(another); + + assertFalse(one.hasNext()); + assertFalse(another.hasNext()); + assertNotEquals(one, another); + } + + /** + * test checking that forEachRemaining resumes at the current position rather than at + * the first permutation, the already returned ones being no longer remaining, and + * that it leaves the iterator exhausted. + */ + @Test + void testForEachRemainingResumesFromCurrentPosition() { + final Iterator> permutationIterator = makeObject(); + final List> permutations = new ArrayList<>(); + + assertEquals(Arrays.asList('A', 'B', 'C'), permutationIterator.next()); + assertEquals(Arrays.asList('A', 'C', 'B'), permutationIterator.next()); + + permutationIterator.forEachRemaining(permutations::add); + + assertEquals(Arrays.asList( + Arrays.asList('B', 'A', 'C'), + Arrays.asList('B', 'C', 'A'), + Arrays.asList('C', 'A', 'B'), + Arrays.asList('C', 'B', 'A')), permutations); + + assertFalse(permutationIterator.hasNext()); + } + + /** + * test checking that forEachRemaining hands the permutations to the action in the + * same lexicographical order as next() returns them. + */ + @Test + void testForEachRemainingYieldsLexicographicOrder() { + final Iterator> permutationIterator = makeObject(); + final List> permutations = new ArrayList<>(); + + permutationIterator.forEachRemaining(permutations::add); + + assertEquals(Arrays.asList( + Arrays.asList('A', 'B', 'C'), + Arrays.asList('A', 'C', 'B'), + Arrays.asList('B', 'A', 'C'), + Arrays.asList('B', 'C', 'A'), + Arrays.asList('C', 'A', 'B'), + Arrays.asList('C', 'B', 'A')), permutations); + + assertFalse(permutationIterator.hasNext()); + } + + /** + * test checking the documented behavior that the hash code changes as the iterator + * advances, and stays consistent with equals: iterators at the same position hash + * alike again. + */ + @Test + void testHashCodeChangesAsIteratorAdvances() { + final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + + assertEquals(one.hashCode(), another.hashCode()); + + one.next(); + assertNotEquals(one.hashCode(), another.hashCode()); + + another.next(); + assertEquals(one.hashCode(), another.hashCode()); + } + + @Test + void testHashCodeForDifferentComparators() { + final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); + final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(7)); + + assertNotEquals(one.hashCode(), another.hashCode()); + } + @Test void testHashCodeForEqualCollections() { final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); @@ -262,6 +413,76 @@ void testHashCodeForEqualCollectionsAndComparators() { assertEquals(one.hashCode(), another.hashCode()); } + /** + * test checking that exhausted iterators hash alike whatever they were built from. + * Equality is defined on the comparator and the next permutation, as documented on + * {@link LexicographicPermutationIterator#equals(Object)}, and the input collection + * is not part of it; two exhausted iterators are therefore equal even when they were + * built from different collections, which makes the equal hash codes required rather + * than incidental. + */ + @Test + void testEqualsAndHashCodeForExhaustedIterators() { + final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); + final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('X', 'Y', 'Z')); + + exhaust(one); + exhaust(another); + + assertEquals(one, another); + assertEquals(one.hashCode(), another.hashCode()); + } + + /** + * test checking that the lists handed out by next() belong to the caller: the next + * permutation is computed and copied before the current one is returned, so mutating + * a returned list, structurally or not, leaves the remaining iteration untouched. + */ + @Test + void testMutatingReturnedListDoesNotAffectIteration() { + final Iterator> permutationIterator = makeObject(); + + final List first = permutationIterator.next(); + assertEquals(Arrays.asList('A', 'B', 'C'), first); + first.set(0, 'Z'); + first.add('Q'); + + // the damage is not merely deferred by one step, so mutate the next one too + final List second = permutationIterator.next(); + assertEquals(Arrays.asList('A', 'C', 'B'), second); + second.clear(); + + assertEquals(Arrays.asList('B', 'A', 'C'), permutationIterator.next()); + assertEquals(Arrays.asList('B', 'C', 'A'), permutationIterator.next()); + assertEquals(Arrays.asList('C', 'A', 'B'), permutationIterator.next()); + assertEquals(Arrays.asList('C', 'B', 'A'), permutationIterator.next()); + + assertFalse(permutationIterator.hasNext()); + } + + /** + * test checking the first permutation specifically, it being the one the constructor + * builds rather than next(): it must be a list of the iterator's own rather than the + * given collection, so that later changes to that collection never reach the + * iteration. The two copies live in different places, so losing one of them would + * corrupt only part of the sequence. + */ + @Test + void testMutatingSourceCollectionDoesNotAffectIteration() { + final List source = new ArrayList<>(Arrays.asList('A', 'B', 'C')); + final Iterator> permutationIterator = new LexicographicPermutationIterator<>(source); + + source.set(0, 'X'); + source.add('Y'); + + final List first = permutationIterator.next(); + assertNotSame(source, first); + assertEquals(Arrays.asList('A', 'B', 'C'), first); + + // the copy is no view of the collection either, so iteration carries on intact + assertEquals(Arrays.asList('A', 'C', 'B'), permutationIterator.next()); + } + @Test void testNonComparableElementsThrow() { final Iterator>> permutationIterator = new LexicographicPermutationIterator<>( @@ -273,6 +494,12 @@ void testNonComparableElementsThrow() { assertThrows(ClassCastException.class, permutationIterator::next); } + @Test + void testNullCollectionThrows() { + assertThrows(NullPointerException.class, () -> new LexicographicPermutationIterator<>(null)); + assertThrows(NullPointerException.class, () -> new LexicographicPermutationIterator<>(null, Comparator.reverseOrder())); + } + @Test void testPermutationException() { final Iterator> permutationIterator = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); From fd2e27ed467920a237a11eae1cc1b6b881f17bdc Mon Sep 17 00:00:00 2001 From: Igor Rudenko Date: Wed, 26 Aug 2026 01:54:44 +0300 Subject: [PATCH 5/5] Remove equals and hashCode --- .../LexicographicPermutationIterator.java | 32 ---- .../LexicographicPermutationIteratorTest.java | 180 ------------------ 2 files changed, 212 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java index aad17a3794..aa4546aa44 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/LexicographicPermutationIterator.java @@ -158,38 +158,6 @@ public void remove() { throw new UnsupportedOperationException("remove() is not supported"); } - /** - * Compares this iterator to another for equality. Two iterators are equal when - * they use equal comparators and are positioned at an equal next permutation. - * - * @param o The object to compare to this instance - * @return true if the given object is an equal iterator, otherwise false - */ - @Override - public boolean equals(final Object o) { - if (this == o) { - return true; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - final LexicographicPermutationIterator that = (LexicographicPermutationIterator) o; - return Objects.equals(comparator, that.comparator) && Objects.equals(nextPermutation, that.nextPermutation); - } - - /** - * Returns a hash code consistent with {@link #equals(Object)}. Note that the - * hash code changes as the iterator advances. - * - * @return A hash code for this instance - */ - @Override - public int hashCode() { - return Objects.hash(comparator, nextPermutation); - } - /** * Compares two elements using the comparator, or their natural ordering if no * comparator was supplied. diff --git a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java index 38065909a8..7125b62188 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java +++ b/src/test/java/org/apache/commons/collections4/iterators/LexicographicPermutationIteratorTest.java @@ -16,10 +16,8 @@ */ package org.apache.commons.collections4.iterators; -import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -42,45 +40,6 @@ */ class LexicographicPermutationIteratorTest extends AbstractIteratorTest> { - /** - * A comparator that orders nothing, identified only by an id, used to check that - * equal comparators make equal iterators. - * - * @param the type of the objects compared - */ - private static final class CustomComparator implements Comparator { - - private final int id; - - CustomComparator(final int id) { - this.id = id; - } - - @Override - public int compare(final T o1, final T o2) { - return 0; - } - - @Override - public boolean equals(final Object o) { - if (this == o) { - return true; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - final CustomComparator cmp = (CustomComparator) o; - return id == cmp.id; - } - - @Override - public int hashCode() { - return id; - } - } - /** * A value holder that deliberately does not implement {@link Comparable}, used to * check that a supplied comparator is honored. @@ -124,17 +83,6 @@ public int hashCode() { protected List testList; - /** - * Advances the given iterator until it holds no further permutation. - * - * @param iterator the iterator to exhaust - */ - private static void exhaust(final Iterator iterator) { - while (iterator.hasNext()) { - iterator.next(); - } - } - @Override public LexicographicPermutationIterator makeEmptyIterator() { return new LexicographicPermutationIterator<>(new ArrayList<>()); @@ -259,71 +207,6 @@ void testEmptyCollection() { assertFalse(permutationIterator.hasNext()); } - @Test - void testEqualsForDifferentComparators() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(7)); - final Iterator> natural = new LexicographicPermutationIterator<>(emptyList()); - - assertNotEquals(one, another); - assertNotEquals(one, natural); - } - - @Test - void testEqualsForDifferentPositions() { - final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); - final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); - - assertEquals(one, another); - - // advancing one of them leaves them at different permutations - one.next(); - assertNotEquals(one, another); - - // advancing the other brings them back to the same permutation - another.next(); - assertEquals(one, another); - - // an exhausted iterator differs from one still holding a permutation - one.next(); - assertFalse(one.hasNext()); - assertNotEquals(one, another); - } - - @Test - void testEqualsForEqualCollections() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList()); - - assertEquals(one, another); - } - - @Test - void testEqualsForEqualCollectionsAndComparators() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - - assertEquals(one, another); - } - - /** - * test checking that the comparator keeps being compared once both iterators are - * exhausted, so that two iterators which will both emit nothing are still unequal. - * The position is spent, but it is not the only part of the equality contract. - */ - @Test - void testEqualsForExhaustedIteratorsWithDifferentComparators() { - final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B'), new CustomComparator<>(42)); - final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B'), new CustomComparator<>(7)); - - exhaust(one); - exhaust(another); - - assertFalse(one.hasNext()); - assertFalse(another.hasNext()); - assertNotEquals(one, another); - } - /** * test checking that forEachRemaining resumes at the current position rather than at * the first permutation, the already returned ones being no longer remaining, and @@ -370,69 +253,6 @@ void testForEachRemainingYieldsLexicographicOrder() { assertFalse(permutationIterator.hasNext()); } - /** - * test checking the documented behavior that the hash code changes as the iterator - * advances, and stays consistent with equals: iterators at the same position hash - * alike again. - */ - @Test - void testHashCodeChangesAsIteratorAdvances() { - final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); - final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); - - assertEquals(one.hashCode(), another.hashCode()); - - one.next(); - assertNotEquals(one.hashCode(), another.hashCode()); - - another.next(); - assertEquals(one.hashCode(), another.hashCode()); - } - - @Test - void testHashCodeForDifferentComparators() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(7)); - - assertNotEquals(one.hashCode(), another.hashCode()); - } - - @Test - void testHashCodeForEqualCollections() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList()); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList()); - - assertEquals(one.hashCode(), another.hashCode()); - } - - @Test - void testHashCodeForEqualCollectionsAndComparators() { - final Iterator> one = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - final Iterator> another = new LexicographicPermutationIterator<>(emptyList(), new CustomComparator<>(42)); - - assertEquals(one.hashCode(), another.hashCode()); - } - - /** - * test checking that exhausted iterators hash alike whatever they were built from. - * Equality is defined on the comparator and the next permutation, as documented on - * {@link LexicographicPermutationIterator#equals(Object)}, and the input collection - * is not part of it; two exhausted iterators are therefore equal even when they were - * built from different collections, which makes the equal hash codes required rather - * than incidental. - */ - @Test - void testEqualsAndHashCodeForExhaustedIterators() { - final Iterator> one = new LexicographicPermutationIterator<>(Arrays.asList('A', 'B')); - final Iterator> another = new LexicographicPermutationIterator<>(Arrays.asList('X', 'Y', 'Z')); - - exhaust(one); - exhaust(another); - - assertEquals(one, another); - assertEquals(one.hashCode(), another.hashCode()); - } - /** * test checking that the lists handed out by next() belong to the caller: the next * permutation is computed and copied before the current one is returned, so mutating