Hi Claude and all: Here are a couple of comments on the bloom filter PR.
10,100 ft level comment we do not have to worry about today: Before the release, we might want to split Commons Collection into a multi-module project and have the BF code in it own module. The _main_ reason for this is that it will allow the dependency on Commons Codecis be a non-optional dependency. Optional dependency are a a bit of a pain IMO, especially when you deal with large stacks. You end up sucking in everything that is optional when you deliver an app because you do not know for certain what's really required at runtime. Closer to the ground: I would make BloomFilter an interface and rename the current BloomFilter class AbstractBloomFilter implements BloomFilter. This will allow the user and maintainer to see what is truly public. Since this is a first cut for Commons Collection, I would consider only making APIs public that must be public. Once released, we MUST maintain binary compatibility within minor releases. Major releases allow us to break this compatibility but these do not happen very often. I do not know what this means yet for BF but it's a thought to consider. This kind of work is made harder due to the current packaging of the BF code. We could consider putting it all in one package if that helps reduce the public API footprint. Q: All the code in main's org.apache.commons.collections4.bloomfilter.hasher.function is only called from test code. Why is that? Nit: Some odd formatting like spaces after open parens: 'toUpperCase( Locale.ROOT)' should be 'toUpperCase(Locale.ROOT)' Gary
