[
https://issues.apache.org/jira/browse/COLLECTIONS-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16962154#comment-16962154
]
Gilles Sadowski commented on COLLECTIONS-728:
---------------------------------------------
bq. implementation (https://github.com/Claudenw/BloomFilter/tree/Hasher)
Nit: There are formatting issues which I mentioned in other reviews.
* Why an _abstract_ class instead of an _interface_ (and _default_ methods)?
* Why pass a {{Hasher}} to the {{Shape}} constructor when only its "name" is
used?
* Why are methods _final_? It's redundant for _private_ ones, and strange for
those name {{estimate...}} (one could imagine that subclasses could have their
own way).
* Why doesn't {{Shape}} keep a reference to {{Hasher}}? That would avoid
having to pass both to e.g. method {{contains}} in {{BloomFilter}} (and would
make is unnecessary to check their consistency).
* I don't get the {{reset()}} method in {{HasherFactory}}. At first sight,
this class should be immutable.
* In {{DynamicHasher}}, it seems that the "lock" construct could be
advantageously replaced with a builder (i.e. the returned {{Hasher}} instance
would only offer the {{getBits()}} API.
* It seems strange to define the {{Hasher}} interface inside {{HasherFactory}}
(IMO that should be the other way around).
* What is the difference between {{StaticHasher}} and {{DynamicHasher}}?
I'm afraid that the review is not exhaustive; again, I have a hard time
figuring out the fundamental (design) structure from convenience features.
> BloomFilter contribution
> ------------------------
>
> Key: COLLECTIONS-728
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-728
> Project: Commons Collections
> Issue Type: Task
> Reporter: Claude Warren
> Priority: Minor
> Attachments: BF_Func.md, BloomFilter.java, BloomFilterI2.java,
> Usage.md
>
>
> Contribution of BloomFilter library comprising base implementation and gated
> collections.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
