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

Reply via email to