[COLLECTIONS-855] Fixed hashing calculation as per report (#501)

* Fixed hashing calculation as per report * fixed up processIndices and simplified code * removed testing check * Put back the k>=bits check * removed blank line
apache · Jun 12, 2024 · a277a2a · a277a2a
1 parent 3fcfbcf
commit a277a2a
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 22 deletions.
diff --git a/src/main/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasher.java b/src/main/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasher.java
@@ -167,42 +167,48 @@ public boolean processIndices(final IntPredicate consumer) {
                 // hash[i] = ( h1(x) - i*h2(x) - (i*i*i - i)/6 ) wrapped in [0, bits)
 
                 int index = BitMaps.mod(initial, bits);
+                if (!consumer.test(index)) {
+                    return false;
+                }
                 int inc = BitMaps.mod(increment, bits);
 
                 final int k = shape.getNumberOfHashFunctions();
-                if (k > bits) {
-                    for (int j = k; j > 0;) {
-                        // handle k > bits
-                        final int block = Math.min(j, bits);
-                        j -= block;
-                        for (int i = 0; i < block; i++) {
-                            if (!consumer.test(index)) {
-                                return false;
-                            }
-                            // Update index and handle wrapping
-                            index -= inc;
-                            index = index < 0 ? index + bits : index;
-
-                            // Incorporate the counter into the increment to create a
-                            // tetrahedral number additional term, and handle wrapping.
-                            inc -= i;
-                            inc = inc < 0 ? inc + bits : inc;
-                        }
-                    }
-                } else {
-                    for (int i = 0; i < k; i++) {
+
+                if (k >= bits) {
+                    // the tetraheadral incrementer.  We need to ensure that this
+                    // number does not exceed bits-1 or we may end up with an index > bits.
+                    int tet = 1;
+                    for (int i = 1; i < k; i++) {
+                        // Update index and handle wrapping
+                        index -= inc;
+                        index = index < 0 ? index + bits : index;
                         if (!consumer.test(index)) {
                             return false;
                         }
+
+                        // Incorporate the counter into the increment to create a
+                        // tetrahedral number additional term, and handle wrapping.
+                        inc -= tet;
+                        inc = inc < 0 ? inc + bits : inc;
+                        if (++tet == bits) {
+                            tet = 0;
+                        }
+                    }
+                } else {
+                    for (int i = 1; i < k; i++) {
                         // Update index and handle wrapping
                         index -= inc;
                         index = index < 0 ? index + bits : index;
+                        if (!consumer.test(index)) {
+                            return false;
+                        }
 
                         // Incorporate the counter into the increment to create a
                         // tetrahedral number additional term, and handle wrapping.
                         inc -= i;
                         inc = inc < 0 ? inc + bits : inc;
                     }
+
                 }
                 return true;
             }

diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java
@@ -25,7 +25,7 @@
  * Tests the {@link EnhancedDoubleHasher}.
  */
 public class EnhancedDoubleHasherTest extends AbstractHasherTest {
-    int[] expected = {1, 0, 71, 71, 1, 6, 15, 29, 49, 4, 39, 11, 65, 58, 63, 9, 41};
+    int[] expected = {1, 0, 0, 2, 7, 16, 30, 50, 5, 40, 12, 66, 59, 64, 10, 42, 17};
 
     @Override
     protected Hasher createEmptyHasher() {