Le dim. 29 déc. 2019 à 15:30, Claude Warren <cla...@xenei.com> a écrit : > > If the Shape class (BloomFilter.Shape) is extracted from the BloomFilter > interface and made a stand-alone class would the name change or is the fact > that it is in the o.a.c.c.bloomfilter package enough? > > I prefer the name Shape to BloomFilterShape.
If "Shape" is only used for "BloomFilter" (as the suggestion above seems to indicate, why not declare it as a sub-interface: ---CUT--- public interface BloomFilter { // ... public interface Shape { // ... } } ---CUT--- ? Regards, Gilles > > Claude > > On Sat, Dec 28, 2019 at 9:06 AM Claude Warren <cla...@xenei.com> wrote: > > > Once the interface is extracted and reduced to the minimum necessary the > > following methods are removed: > > > > orCardinality() -- we have andCardinality() and xorCardinality() this was > > included for completeness. > > > > isFull() -- true if all the bits in the vector are on. A convenience > > method used to short circuit some logic. > > > > I think these 2 methods should go into the BloomFilter interface. > > > > > > Set operations: > > > > jaccardSimilarity -- a measure of similarity in sets with range [0,1] > > > > jaccardDistance -- defined as 1 - jaccardSimilarity > > > > cosineSimilarity -- a measure of similarity with range [0,1] > > > > cosineDistance -- defined as 1 - cosineSimilarity > > > > estimateSize -- Set operation estimate of the number of items that were > > placed in the filter. (Requires Shape) > > > > estimateUnionSize -- Set operation estimate of the number of items that > > would be represented by the merging of two filters. (Requires Shape) > > > > estimateIntersectionSize -- Set operations estimate of the number of items > > in the intersection. (Requires Shape) > > > > Perhaps it makes sense to move the Set operations into their own class > > with static methods? The set operations all operate on 2 Bloom filters. > > Moving them would clarify the AbstractBloomFilter class. > > > > Claude > > > > > > On Sat, Dec 28, 2019 at 2:01 AM Gary Gregory <garydgreg...@gmail.com> > > wrote: > > > >> On Fri, Dec 27, 2019 at 11:36 AM Claude Warren <cla...@xenei.com> wrote: > >> > >> > On Fri, Dec 27, 2019 at 1:02 AM Gary Gregory <garydgreg...@gmail.com> > >> > wrote: > >> > > >> > > 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. > >> > > > >> > > >> > I have done this (not checked in) but the net effect is that all the > >> public > >> > methods in the AbstractBloomFilter class are reflected in the > >> BloomFilter > >> > interface and the Shape class has moved to the Interface as well. > >> > > >> > >> OK, I think I like BloomFilter as an interface especially since it is used > >> as an argument in methods. I'll leave it to you as to whether Shape needs > >> to be folded in. I did notice that Shape is an argument in a few places. > >> Might we loose some focus and abstraction by folding Shape into the > >> BloomFilter interface? > >> > >> > >> > I can remove several methods from BloomFilter that are not strictly > >> > necessary for this code to function. I am and have been reticent to do > >> so > >> > since there are a number of home-grown libraries used by various > >> > researchers that provide these functions. But if that is what it takes > >> to > >> > get this out the door it will be done. > >> > > >> > >> Once you have BloomFilter as an interface with an implementing class, it > >> might make it much clearer as to what really belongs in the interface. The > >> handy methods can stay in the abstract class of course. > >> > >> > >> > > >> > > 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. > >> > > > >> > > >> > I think that putting all the pieces into a single package will muddy the > >> > waters a bit. The classes in o.a.c.c.bloomfilter are high level classes > >> > and used wherever the Bloom filter code is used. Objects in the > >> > o.a.c.c.bloomfilter.hasher classes are Hashers and are really only > >> selected > >> > when the application developer determines which hasher is best suited > >> for > >> > the environment. Finally o.a.c.c.bloomfilter.hasher.function are > >> > HasherFunction implementations. These can be used or special cases > >> built > >> > by the implementer as necessary. Perhaps I am being a bit too pedantic > >> but > >> > I do not think it makes sense to merge them into a single package. > >> > > >> > >> Let's leave it as is then. > >> > >> Side note: I'll need to release Commons Codec 1.14 before we can bring in > >> this PR. I hope to do this after Commons VFS has gone through its own > >> release cycle (which should be done in 48 hours or so if all goes well.) > >> Well, we could bring in the PR but depending on a SNAPSHOT build is > >> usually > >> not a good idea unless it is truly short term. > >> > >> Thank you, > >> Gary > >> > >> > >> > > >> > > >> > > > >> > > Q: All the code in main's > >> > > org.apache.commons.collections4.bloomfilter.hasher.function is only > >> > called > >> > > from test code. Why is that? > >> > > > >> > > >> > They are pluggable modules. When a developer uses them they an > >> instance is > >> > created and it is passed to the BloomFilter.Shape() constructor as well > >> as > >> > to the various Hasher constructors. For example: > >> > > >> > HashFunction hashFunction = new Murmur128x86Cyclic(); > >> > Shape bloomFilterConfig = new Shape( hashFunction, 3, 1.0 / 100000); > >> > > >> > > >> > > > >> > > Nit: Some odd formatting like spaces after open parens: 'toUpperCase( > >> > > Locale.ROOT)' should be 'toUpperCase(Locale.ROOT)' > >> > > > >> > > Gary > >> > > > >> > > >> > > >> > -- > >> > I like: Like Like - The likeliest place on the web > >> > <http://like-like.xenei.com> > >> > LinkedIn: http://www.linkedin.com/in/claudewarren > >> > > >> > > > > > > -- > > I like: Like Like - The likeliest place on the web > > <http://like-like.xenei.com> > > LinkedIn: http://www.linkedin.com/in/claudewarren > > > > > -- > I like: Like Like - The likeliest place on the web > <http://like-like.xenei.com> > LinkedIn: http://www.linkedin.com/in/claudewarren --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org