Claudenw commented on code in PR #358:
URL:
https://github.com/apache/commons-collections/pull/358#discussion_r1020765745
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -245,16 +268,43 @@ default int estimateUnion(final BloomFilter other) {
/**
* Estimates the number of items in the intersection of this Bloom filter
with the other bloom filter.
*
- * <p>By default this is the {@code estimateN() + other.estimateN() -
estimateUnion(other)} </p>
+ * <p>This method produces estimate is roughly equivalent to the number of
unique Hashers that have been merged into both
+ * of the filters by rounding the value from the calculation described in
the {@link Shape} class javadoc.</p>
*
- * <p>This produces estimate is roughly equivalent to the number of unique
Hashers that have been merged into both
- * of the filters.</p>
+ * <p><em>{@code estimateIntersection} should only be called with Bloom
filters of the same Shape. If called on Bloom
+ * filters of differing shape this method is not symmetric. If {@code
other} has more bits an {@code IllegalArgumentException}
+ * may be thrown.</em></p>
*
* @param other The other Bloom filter
- * @return an estimate of the number of items in the intersection.
+ * @return an estimate of the number of items in the intersection. If the
calculated estimate is larger than Integer.MAX_VALUE then MAX_VALUE is returned.
+ * @throws IllegalArgumentException if the estimated N for the union of
the filters is infinite.
+ * @see #estimateN()
+ * @see Shape
*/
default int estimateIntersection(final BloomFilter other) {
Objects.requireNonNull(other, "other");
- return estimateN() + other.estimateN() - estimateUnion(other);
+ double eThis = getShape().estimateN(cardinality());
+ double eOther = getShape().estimateN(other.cardinality());
+ long estimate = 0L;
+ if (Double.isInfinite(eThis) && Double.isInfinite(eOther)) {
+ // if both are infinite the union is infinite and we return
Integer.MAX_VALUE
+ return Integer.MAX_VALUE;
+ }
+ // if one is infinite the intersection is the other.
+ if (Double.isInfinite(eThis)) {
+ estimate = Math.round(eOther);
+ } else if (Double.isInfinite(eOther)) {
+ estimate = Math.round(eThis);
+ } else {
+ BloomFilter union = this.copy();
+ union.merge(other);
+ double eUnion = getShape().estimateN(union.cardinality());
+ if (Double.isInfinite(eUnion)) {
+ throw new IllegalArgumentException("The estimated N for the
union of the filters is infinite");
+ }
+ // all estimated values are small values greater than 0 but less
that number of bits
Review Comment:
Modified to specify exact upper limit
--
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]
