[commons-numbers] 09/09: Fixed test coverage in CombinationsTest

[ah...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit dc0071f91f110f7e7974f568ff54bf77f13e72ac
Author: Alex Herbert <ah...@apache.org>
AuthorDate: Sat Apr 4 22:09:31 2020 +0100

    Fixed test coverage in CombinationsTest
---
 .../numbers/combinatorics/Combinations.java        | 10 +---
 .../numbers/combinatorics/CombinationsTest.java    | 61 +++++++++++++++++++---
 2 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/Combinations.java b/commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/Combinations.java
index 86ef55a..6d19160 100644
--- a/commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/Combinations.java
+++ b/commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/Combinations.java
@@ -123,8 +123,7 @@ public final class Combinations implements Iterable<int[]> {
      * Combinations, D. Knuth, 2004.</p>
      * <p>
      * The degenerate cases {@code k == 0} and {@code k == n} are NOT handled by this
-     * implementation.  If constructor arguments satisfy {@code k == 0}
-     * or {@code k >= n}, no exception is generated, but the iterator is empty.
+     * implementation. It is assumed that {@code n > k > 0}.
      * </p>
      */
     private static class LexicographicIterator implements Iterator<int[]> {
@@ -151,8 +150,7 @@ public final class Combinations implements Iterable<int[]> {
          * Construct a CombinationIterator to enumerate {@code k}-sets from a set
          * of size {@code n}.
          * <p>
-         * NOTE: If {@code k === 0} or {@code k >= n}, the Iterator will be empty
-         * (that is, {@link #hasNext()} will return {@code false} immediately.
+         * NOTE: It is assumed that {@code n > k > 0}.
          * </p>
          *
          * @param n Size of the set from which subsets are enumerated.
@@ -161,10 +159,6 @@ public final class Combinations implements Iterable<int[]> {
         LexicographicIterator(int n, int k) {
             this.k = k;
             c = new int[k + 3];
-            if (k == 0 || k >= n) {
-                more = false;
-                return;
-            }
             // Initialize c to start with lexicographically first k-set
             for (int i = 1; i <= k; i++) {
                 c[i] = i - 1;
diff --git a/commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/CombinationsTest.java b/commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/CombinationsTest.java
index fba85bb..ca3ead1 100644
--- a/commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/CombinationsTest.java
+++ b/commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/CombinationsTest.java
@@ -17,6 +17,7 @@
 package org.apache.commons.numbers.combinatorics;
 
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 import java.util.Comparator;
 
 import org.junit.jupiter.api.Assertions;
@@ -48,6 +49,7 @@ public class CombinationsTest {
         checkLexicographicIterator(6, 1);
         checkLexicographicIterator(3, 3);
         checkLexicographicIterator(1, 1);
+        checkLexicographicIterator(2, 0);
         checkLexicographicIterator(1, 0);
         checkLexicographicIterator(0, 0);
         checkLexicographicIterator(4, 2);
@@ -55,6 +57,13 @@ public class CombinationsTest {
     }
 
     @Test
+    public void testLexicographicIteratorThrows() {
+        checkLexicographicIteratorThrows(2, 1);
+        // Only 1 combination
+        checkLexicographicIteratorThrows(1, 1);
+    }
+
+    @Test
     public void testLexicographicComparatorWrongIterate1() {
         final int n = 5;
         final int k = 3;
@@ -182,14 +191,54 @@ public class CombinationsTest {
         Assertions.assertEquals(BinomialCoefficient.value(n, k), numIterates);
     }
 
+    /**
+     * Verifies that the iterator throws exceptions when misused.
+     *
+     * @param n Size of the set from which subsets are selected.
+     * @param k Size of the subsets to be enumerated.
+     */
+    private static void checkLexicographicIteratorThrows(int n,
+                                                         int k) {
+        Iterator<int[]> iter = Combinations.of(n, k).iterator();
+
+        // First call
+        iter.next();
+        // Check remove is not supported
+        Assertions.assertThrows(UnsupportedOperationException.class, () -> iter.remove());
+
+        // Consume the rest
+        final long numIterates = BinomialCoefficient.value(n, k);
+        for (long i = 1; i < numIterates; i++) {
+            iter.next();
+        }
+        Assertions.assertThrows(NoSuchElementException.class, () -> iter.next());
+    }
+
     @Test
-    public void testCombinationsPrecondition1() {
-        Assertions.assertThrows(IllegalArgumentException.class,
-            () -> Combinations.of(4, 5));
+    public void testBinomialCoefficientKLargerThanN() {
+        Assertions.assertThrows(CombinatoricsException.class,
+            () -> Combinations.of(4, 5)
+        );
     }
+
     @Test
-    public void testCombinationsPrecondition2() {
-        Assertions.assertThrows(IllegalArgumentException.class,
-            () -> Combinations.of(-1, -2));
+    public void testBinomialCoefficientNegativeN() {
+        Assertions.assertThrows(CombinatoricsException.class,
+            () -> Combinations.of(-1, 1)
+        );
+    }
+
+    @Test
+    public void testBinomialCoefficientNegativeK() {
+        Assertions.assertThrows(CombinatoricsException.class,
+            () -> Combinations.of(10, -1)
+        );
+    }
+
+    @Test
+    public void testBinomialCoefficientKAboveN() {
+        Assertions.assertThrows(CombinatoricsException.class,
+            () -> Combinations.of(10, 20)
+        );
     }
 }