yf0994 / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

BloomFilter should have an asPredicate() view #1081

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
See summary.

Original issue reported on code.google.com by gak@google.com on 24 Jul 2012 at 6:01

GoogleCodeExporter commented 9 years ago
Or have an asPredicate() instance method.

Original comment by kurt.kluever on 24 Jul 2012 at 6:12

GoogleCodeExporter commented 9 years ago
Issue 1058 has been merged into this issue.

Original comment by kurt.kluever on 24 Jul 2012 at 6:41

GoogleCodeExporter commented 9 years ago
Now that I think about it, I have a slight prefer to asPredicate instead of 
implementing Predicate. The OP wanted an immutable BloomFilter...passing around 
a BloomFilter instance (and having people only call apply()) won't prevent them 
from calling put(), so really we should let them view the BloomFilter as a 
Predicate. Then they can safely pass/expose a Predicate<E> and maintain 
immutability.

Let me know if you disagree.

Original comment by kurt.kluever on 24 Jul 2012 at 6:46

GoogleCodeExporter commented 9 years ago
---------- Forwarded message ----------
From: Kevin Bourrillion <kevinb@google.com>
Date: Fri, Jun 24, 2011 at 5:58 PM

On Thu, Jun 23, 2011 at 12:23 PM, Dimitris Andreou <andreou@google.com> wrote:
> About making BloomFilter a Predicate... umm. That feels a little weird, I'm 
not
> sure I can get used to that. I suspect you are not very sure about this 
either,
> right?
>
> I guess the reason that I'm not sure about it is that a BloomFilter, if 
someone
> doesn't know what a BloomFilter is, doesn't make it very obvious what
> BloomFilter#apply does (which side can err), unlike #mightContains, which is a
> good name, users can quickly get what this means even without having seen a
> Bloom filter before.

Glad you bring this up; we should discuss it.  We don't want something to 
implement Predicate if it's sufficiently ambiguous what it means when used as a 
Predicate. I thought "this case is not ambiguous because there's only one 
boolean operation a bloom filter does."  But yeah, that operation itself has 
such subtlety to it, so there's that.

What prompted me to do it: I made a "BloomFilters.screen(Predicate, 
BloomFilter)" utility method, that takes a (presumably expensive) predicate and 
a bloom filter to which every "matching" element has been added; it would 
return a very fast predicate that's functionality equivalent to the input 
predicate. Then, I realized I was reimplementing exactly Predicates.and().  
Upon reflection, if this is the only advantage of having BF implement 
Predicate, it's not enough.  And we probably ought to have such a screen() 
method anyway, because "Predicates.and(filter, expensivePredicate)" is not very 
intention-revealing.

I'll just back this out.

Original comment by cpov...@google.com on 24 Jul 2012 at 6:56

GoogleCodeExporter commented 9 years ago
Hrrrrm. I'm sort of liking the approach of just a BloomFilter.screen(Predicate) 
method?

Original comment by wasserman.louis on 25 Jul 2012 at 7:59

GoogleCodeExporter commented 9 years ago
Unless I'm misunderstanding, screen() doesn't help us get an unmodifiable view 
or immutable BF (which is what the OP wanted). If BF implemented Predicate 
(which is another option we're discussing), then screen() would be an 
additional, unrelated operation we might consider adding (but I'm not sure how 
common that operation would be).

I'm still +1 on the asPredicate() view, but we'll discuss this at API review on 
Thursday.

Original comment by kak@google.com on 25 Jul 2012 at 8:08

GoogleCodeExporter commented 9 years ago
I would like a predicate view of BloomFilter so I can take a collection of 
elements and use a filter method to get the candidates worth looking up in the 
database.

Original comment by emily@soldal.org on 25 Jul 2012 at 8:09

GoogleCodeExporter commented 9 years ago
As I stated offline, if we want an unmodifiable view of a BF, we should just 
make an unmodifiable view of a BF.  I don't think that an unmodifiable view of 
a _single_ method which retains none of the BF context is sufficient.  

Side note: BF.copy() makes defensive copying really easy.

I think that the predicate view should be judged more on the merits of the use 
cases that Louis and Emily describe.

IMO, I'd rather write:
ImmutableSet<Element> possibleElements = 
someFluentIterable.filter(bloomFilter).toImmutableSet();
than
ImmutableSet<Element> possibleElements = 
someFluentIterable.filter(bloomFilter.asPredicate()).toImmutableSet();

But obviously there's not a huge difference here.

Original comment by gak@google.com on 25 Jul 2012 at 4:29

GoogleCodeExporter commented 9 years ago
Either BloomFilter implementing Predicate or having an asPredicate view works 
for me.  I have several of these filters which I ultimately combine using 
Predicates.or().

Original comment by arg...@gmail.com on 25 Jul 2012 at 4:34

GoogleCodeExporter commented 9 years ago
Hmmmmm.  I argue the following: screen is usually what you want to do with a 
BloomFilter, just because it's usually used as a "first pass."  But in the 
rarer case that you want an asPredicate view, that's equivalent to 
screen(Predicates.alwaysTrue()).

Original comment by wasserman.louis on 26 Jul 2012 at 9:33

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08