[GitHub] [commons-collections] aherbert commented on a diff in pull request #328: Collections 827: Add tests using or, and and xor with different length filters.

[GitBox <gi...@apache.org>]

aherbert commented on code in PR #328:
URL: https://github.com/apache/commons-collections/pull/328#discussion_r941542308


##########
src/test/java/org/apache/commons/collections4/bloomfilter/SetOperationsTest.java:
##########
@@ -222,12 +222,26 @@ public final void testOrCardinality() {
         filter2 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 64, 69 }));
         assertEquals(4, SetOperations.orCardinality(filter1, filter2));
         assertEquals(4, SetOperations.orCardinality(filter2, filter1));
+    }
 
-        Shape bigShape = Shape.fromKM(3, 192);
-        filter1 = new SparseBloomFilter(bigShape, IndexProducer.fromIndexArray(new int[] { 1, 63, 185}));
-        filter2 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 63, 69 }));
+    @Test
+    public final void testOrCardinalityWithDifferentLengthFilters() {
+        Shape shape = Shape.fromKM(3, 128);
+        Shape shape2 = Shape.fromKM(3, 192);
+        SparseBloomFilter filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 1, 63, 64 }));
+        SparseBloomFilter filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
+        assertEquals(5, SetOperations.orCardinality(filter1, filter2));
+        assertEquals(5, SetOperations.orCardinality(filter2, filter1));
+
+        filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 1, 63 }));
+        filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
         assertEquals(5, SetOperations.orCardinality(filter1, filter2));
         assertEquals(5, SetOperations.orCardinality(filter2, filter1));
+
+        filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 63 }));
+        filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
+        assertEquals(4, SetOperations.orCardinality(filter1, filter2));
+        assertEquals(4, SetOperations.orCardinality(filter2, filter1));

Review Comment:
   I think we can reduce the amount of duplicate code with a custom assertion:
   ```Java
           assertSymmetricOperation(4, SetOperations::orCardinality, filter1, filter2);
       }
   
       private static void assertSymmetricOperation(int expected, ToIntBiFunction<BloomFilter, BloomFilter> operation,
               BloomFilter filter1, BloomFilter filter2) {
           assertEquals(expected, operation.applyAsInt(filter1, filter2), "op(filter1, filter2)");
           assertEquals(expected, operation.applyAsInt(filter2, filter1), "op(filter2, filter1)");
       }
   ```
   
   This ensures you only have one place where you specify the expected value and where filter 1 and 2 are swapped as arguments to the assertion.



-- 
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: issues-unsubscribe@commons.apache.org

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