From 0d10d917d71072dacba090aa864090523ff5502c Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 20 Nov 2020 02:31:25 +0000 Subject: [PATCH] Removed constructors that set initial values (#162) * Removed constructors that set initial values * Fixed testing errors. --- .../bloomfilter/ArrayCountingBloomFilter.java | 22 --- .../bloomfilter/BitSetBloomFilter.java | 12 -- .../ArrayCountingBloomFilterTest.java | 149 ++++++++++-------- .../bloomfilter/BitSetBloomFilterTest.java | 4 +- 4 files changed, 86 insertions(+), 101 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java b/src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java index f2420059c..b1b7f40fd 100644 --- a/src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java +++ b/src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java @@ -140,28 +140,6 @@ public class ArrayCountingBloomFilter extends AbstractBloomFilter implements Cou counts = new int[shape.getNumberOfBits()]; } - /** - * Constructs a counting Bloom filter from a hasher and a shape. - * - *

The filter will be equal to the result of merging the hasher with an empty - * filter; specifically duplicate indexes in the hasher are ignored. - * - * @param hasher the hasher to build the filter from - * @param shape the shape of the filter - * @throws IllegalArgumentException if the hasher cannot generate indices for - * the shape - * @see #merge(Hasher) - */ - public ArrayCountingBloomFilter(final Hasher hasher, final Shape shape) { - super(shape); - // Given the filter is empty we can optimise the operation of merge(hasher) - verifyHasher(hasher); - // Delay array allocation until after hasher is verified - counts = new int[shape.getNumberOfBits()]; - // All counts are zero. Ignore duplicates by initialising to 1 - hasher.iterator(shape).forEachRemaining((IntConsumer) idx -> counts[idx] = 1); - } - @Override public int cardinality() { int size = 0; diff --git a/src/main/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilter.java b/src/main/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilter.java index e948d334e..de55cbe93 100644 --- a/src/main/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilter.java +++ b/src/main/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilter.java @@ -36,18 +36,6 @@ public class BitSetBloomFilter extends AbstractBloomFilter { */ private final BitSet bitSet; - /** - * Constructs a BitSetBloomFilter from a hasher and a shape. - * - * @param hasher the Hasher to use. - * @param shape the desired shape of the filter. - */ - public BitSetBloomFilter(final Hasher hasher, final Shape shape) { - this(shape); - verifyHasher(hasher); - hasher.iterator(shape).forEachRemaining((IntConsumer) bitSet::set); - } - /** * Constructs an empty BitSetBloomFilter. * diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilterTest.java index 7bd5e5d50..268471331 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilterTest.java @@ -37,6 +37,19 @@ import org.junit.Test; */ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { + /** + * Function to convert int arrays to BloomFilters for testing. + */ + private Function converter = new Function() { + + @Override + public BloomFilter apply(int[] counts) { + BloomFilter testingFilter = new BitSetBloomFilter(shape); + testingFilter.merge(new FixedIndexesTestHasher(shape, counts)); + return testingFilter; + } + }; + @Override protected ArrayCountingBloomFilter createEmptyFilter(final Shape shape) { return new ArrayCountingBloomFilter(shape); @@ -44,7 +57,9 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Override protected ArrayCountingBloomFilter createFilter(final Hasher hasher, final Shape shape) { - return new ArrayCountingBloomFilter(hasher, shape); + ArrayCountingBloomFilter result = new ArrayCountingBloomFilter(shape); + result.merge( hasher ); + return result; } private ArrayCountingBloomFilter createFromCounts(int[] counts) { @@ -111,8 +126,12 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { // Some indexes with duplicates final Hasher hasher = new FixedIndexesTestHasher(shape, 1, 2, 5); final ArrayCountingBloomFilter bf = createFilter(hasher, shape); - assertFalse(bf.contains(new BitSetBloomFilter(new FixedIndexesTestHasher(shape, 3, 4), shape))); - assertTrue(bf.contains(new BitSetBloomFilter(new FixedIndexesTestHasher(shape, 2, 5), shape))); + BitSetBloomFilter testingFilter = new BitSetBloomFilter(shape); + testingFilter.merge( new FixedIndexesTestHasher(shape, 3, 4)); + assertFalse(bf.contains(testingFilter)); + testingFilter = new BitSetBloomFilter(shape); + testingFilter.merge( new FixedIndexesTestHasher(shape, 2, 5)); + assertTrue(bf.contains(testingFilter)); } /** @@ -122,7 +141,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void mergeTest_Counts_CountingBloomFilter() { assertMerge(counts -> createFilter(new FixedIndexesTestHasher(shape, counts), shape), - BloomFilter::merge); + BloomFilter::merge); } /** @@ -130,8 +149,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { */ @Test public void mergeTest_Counts_BloomFilter() { - assertMerge(counts -> new BitSetBloomFilter(new FixedIndexesTestHasher(shape, counts), shape), - BloomFilter::merge); + assertMerge(converter, BloomFilter::merge); } /** @@ -140,7 +158,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void mergeTest_Counts_Hasher() { assertMerge(counts -> new FixedIndexesTestHasher(shape, counts), - BloomFilter::merge); + BloomFilter::merge); } /** @@ -149,7 +167,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void mergeTest_Counts_Hasher_Duplicates() { assertMerge(counts -> new FixedIndexesTestHasher(shape, createDuplicates(counts)), - BloomFilter::merge); + BloomFilter::merge); } /** @@ -159,7 +177,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void removeTest_Counts_CountingBloomFilter() { assertRemove(counts -> createFilter(new FixedIndexesTestHasher(shape, counts), shape), - CountingBloomFilter::remove); + CountingBloomFilter::remove); } /** @@ -167,8 +185,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { */ @Test public void removeTest_Counts_BloomFilter() { - assertRemove(counts -> new BitSetBloomFilter(new FixedIndexesTestHasher(shape, counts), shape), - CountingBloomFilter::remove); + assertRemove(converter, CountingBloomFilter::remove); } /** @@ -177,7 +194,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void removeTest_Counts_Hasher() { assertRemove(counts -> new FixedIndexesTestHasher(shape, counts), - CountingBloomFilter::remove); + CountingBloomFilter::remove); } /** @@ -186,7 +203,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void removeTest_Counts_Hasher_Duplicates() { assertRemove(counts -> new FixedIndexesTestHasher(shape, createDuplicates(counts)), - CountingBloomFilter::remove); + CountingBloomFilter::remove); } /** @@ -215,7 +232,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { * @param merge the merge operation */ private void assertMerge(Function converter, - BiPredicate merge) { + BiPredicate merge) { final int[] indexes1 = { 1, 2, 4, 5, 6}; final int[] indexes2 = { 3, 4, 6}; final int[] expected = {0, 1, 1, 1, 2, 1, 2}; @@ -231,7 +248,7 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { * @param remove the remove operation */ private void assertRemove(Function converter, - BiPredicate remove) { + BiPredicate remove) { final int[] indexes1 = { 1, 2, 4, 5, 6}; final int[] indexes2 = { 2, 5, 6}; final int[] expected = {0, 1, 0, 0, 1, 0, 0}; @@ -345,10 +362,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void addTest_Empty() { assertCountingOperation(new int[] {5, 2, 1}, - new int[0], - CountingBloomFilter::add, - true, - new int[] {5, 2, 1}); + new int[0], + CountingBloomFilter::add, + true, + new int[] {5, 2, 1}); } /** @@ -358,10 +375,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void addTest_Counts() { assertCountingOperation(new int[] {5, 2, 1}, - new int[] {0, 6, 4, 1}, - CountingBloomFilter::add, - true, - new int[] {5, 8, 5, 1}); + new int[] {0, 6, 4, 1}, + CountingBloomFilter::add, + true, + new int[] {5, 8, 5, 1}); } /** @@ -371,10 +388,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void addTest_Overflow() { assertCountingOperation(new int[] {5, 2, 1}, - new int[] {0, 6, Integer.MAX_VALUE}, - CountingBloomFilter::add, - false, - new int[] {5, 8, 1 + Integer.MAX_VALUE}); + new int[] {0, 6, Integer.MAX_VALUE}, + CountingBloomFilter::add, + false, + new int[] {5, 8, 1 + Integer.MAX_VALUE}); } /** @@ -383,10 +400,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void subtractTest_Empty() { assertCountingOperation(new int[] {5, 2, 1}, - new int[0], - CountingBloomFilter::subtract, - true, - new int[] {5, 2, 1}); + new int[0], + CountingBloomFilter::subtract, + true, + new int[] {5, 2, 1}); } /** @@ -396,10 +413,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void subtractTest_Counts() { assertCountingOperation(new int[] {5, 9, 1, 1}, - new int[] {0, 2, 1}, - CountingBloomFilter::subtract, - true, - new int[] {5, 7, 0, 1}); + new int[] {0, 2, 1}, + CountingBloomFilter::subtract, + true, + new int[] {5, 7, 0, 1}); } /** @@ -409,10 +426,10 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void subtractTest_Negative() { assertCountingOperation(new int[] {5, 2, 1}, - new int[] {0, 6, 1}, - CountingBloomFilter::subtract, - false, - new int[] {5, -4, 0}); + new int[] {0, 6, 1}, + CountingBloomFilter::subtract, + false, + new int[] {5, -4, 0}); } /** @@ -446,17 +463,17 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void andCardinalityTest_CountingBloomFilter() { assertCardinalityOperation(new int[] {1, 1}, - new int[] {1, 1}, - BloomFilter::andCardinality, - 2); + new int[] {1, 1}, + BloomFilter::andCardinality, + 2); assertCardinalityOperation(new int[] {0, 1, 0, 1, 1, 1, 0, 1, 0}, - new int[] {1, 1, 0, 0, 0, 1}, - BloomFilter::andCardinality, - 2); + new int[] {1, 1, 0, 0, 0, 1}, + BloomFilter::andCardinality, + 2); assertCardinalityOperation(new int[] {1, 1}, - new int[] {0, 0, 1, 1, 1}, - BloomFilter::andCardinality, - 0); + new int[] {0, 0, 1, 1, 1}, + BloomFilter::andCardinality, + 0); } /** @@ -466,17 +483,17 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void orCardinalityTest_CountingBloomFilter() { assertCardinalityOperation(new int[] {1, 1}, - new int[] {1, 1}, - BloomFilter::orCardinality, - 2); + new int[] {1, 1}, + BloomFilter::orCardinality, + 2); assertCardinalityOperation(new int[] {0, 1, 0, 1, 1, 1, 0, 1, 0}, - new int[] {1, 1, 0, 0, 0, 1}, - BloomFilter::orCardinality, - 6); + new int[] {1, 1, 0, 0, 0, 1}, + BloomFilter::orCardinality, + 6); assertCardinalityOperation(new int[] {1, 1}, - new int[] {0, 0, 1, 1, 1}, - BloomFilter::orCardinality, - 5); + new int[] {0, 0, 1, 1, 1}, + BloomFilter::orCardinality, + 5); } /** @@ -486,17 +503,17 @@ public class ArrayCountingBloomFilterTest extends AbstractBloomFilterTest { @Test public void xorCardinalityTest_CountingBloomFilter() { assertCardinalityOperation(new int[] {1, 1}, - new int[] {1, 1}, - BloomFilter::xorCardinality, - 0); + new int[] {1, 1}, + BloomFilter::xorCardinality, + 0); assertCardinalityOperation(new int[] {0, 1, 0, 1, 1, 1, 0, 1, 0}, - new int[] {1, 1, 0, 0, 0, 1}, - BloomFilter::xorCardinality, - 4); + new int[] {1, 1, 0, 0, 0, 1}, + BloomFilter::xorCardinality, + 4); assertCardinalityOperation(new int[] {1, 1}, - new int[] {0, 0, 1, 1, 1}, - BloomFilter::xorCardinality, - 5); + new int[] {0, 0, 1, 1, 1}, + BloomFilter::xorCardinality, + 5); } /** diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilterTest.java index 5c5aef7cb..dae71d54c 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/BitSetBloomFilterTest.java @@ -30,6 +30,8 @@ public class BitSetBloomFilterTest extends AbstractBloomFilterTest { @Override protected BitSetBloomFilter createFilter(final Hasher hasher, final Shape shape) { - return new BitSetBloomFilter(hasher, shape); + BitSetBloomFilter testFilter = new BitSetBloomFilter(shape); + testFilter.merge( hasher ); + return testFilter; } }