Claudenw commented on code in PR #406:
URL: 
https://github.com/apache/commons-collections/pull/406#discussion_r1278265080


##########
src/main/java/org/apache/commons/collections4/bloomfilter/CountingBloomFilter.java:
##########
@@ -121,7 +186,7 @@ default boolean merge(final Hasher hasher) {
     /**
      * Merges the specified index producer into this Bloom filter.
      *
-     * <p>Specifically: all counts for the indexes identified by the {@code 
indexProducer} will be incremented by 1.</p>
+     * <p>Specifically: all cells for the indexes identified by the {@code 
indexProducer} will be incremented by 1.</p>

Review Comment:
   Agree this is incorrect.   I am adding this as a test to the 
AbstractCountingFilterTest.
   
   I think that we should make the following changes.
   
   for counting filters
   
   - merge always increments the cell value by at most 1.
   - remove always decrements the cell value by at most 1.
   - add and subtract as used to increment/decrement by more than 1.
   
   for index producers
   
   - as array always returns the indices in the number and order they are 
produced.
   - move the "unique" method from hashers to index producers. Hashers create 
index producers.  Let the use of the producer determine if it needs to be 
unique.
   
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to