yangxu998 / guava-libraries

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

Wanted in Collections2: Collection<T> filter(Collection<?>, Class<T>) #604

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There is a corollary in Iterables. Was it omitted from Collections2 
intentionally?

I see the warning in the Javadoc for Collections2.filter(Collection, 
Predicate), but it's unclear to me why it says the predicate *must* be 
consistent with equals. The referenced Javadoc for Predicate.apply specifically 
says that consistency with equals is *not* an absolute requirement. I've 
re-read the Javadoc for Collection and nothing there indicates that it has any 
"equals" requirements that would justify a different policy for applying 
predicates than what you have for Iterable.

If the omission was intentional, was it because you consider 
Iterables.filter(Iterable, Class) a mistake? I hope not. It's useful, and a 
corresponding method in Collections2 would be too.

If you decide not to implement the method I'm suggesting, I hope you will at 
least clarify your Javadoc to make it clear why it's missing. Thanks.

Original issue reported on code.google.com by jaredjac...@gmail.com on 14 Apr 2011 at 7:26

GoogleCodeExporter commented 9 years ago
If we drop the consistency requirement, we'd have to drop the optimization in 
FilteredCollection.contains().  So if you filter 10 million Integers down to 
the ~5 million that are odd, and then ask that if it contains 2, it will really 
have to check all those 5 million items.

The main reason there's no Collections2.filter(Collection, Class) is probably 
that we have so few users of the Iterables version of that method (from what we 
can tell), and we get a lot of complaints that it is nearly what users want but 
not quite (they really want something that throws an exception if the wrong 
type is encountered).

Original comment by kevin...@gmail.com on 19 Apr 2011 at 2:22

GoogleCodeExporter commented 9 years ago
Another datapoint on Iterables.filter():

We *do* use Iterables.filter() quite often (65 use in our project), but there 
are indeed many cases were what we really want is something that throws an 
exception if the wrong type is encountered.

I once had to write the following ugly code...

public void doStuff(List<Foo> foos) {
    List<SubFoo> subFoos = ImmutableList.copyOf(Iterables.filter(foos, SubFoo.class));
    warnIfFilterGroupsHaveWrongType(foos, subFoos);
    // ...
}

private void warnIfFoosHaveWrongType(List<Foo> source, List<SubFoo> dest) {
    if (source.size() != dest.size()) {
        Iterable<Foo> foosOfWrongType = Iterables.filter(source, not(instanceOf(SubFoo.class)));
        logger.warn(String.format("Following Foos have the wrong type: %s", foosOfWrongType));
    }
}

So +1 for some kind of "Iterables.filterAndCast()" method.

Original comment by nev...@gmail.com on 19 Apr 2011 at 11:20

GoogleCodeExporter commented 9 years ago
Thanks, Kevin, for the explanation. I wasn't aware of the optimization in 
FilteredCollection.contains(). It's well worth keeping for cases when 
.contains() in the underlying Collection isn't O(1).

I have a proposition that I hope you will consider. Right now, the Javadoc for 
Collections2.filter(Collection, Predicate) says:

"Warning: predicate must be consistent with equals, as documented at 
Predicate.apply. Do not provide a predicate such as 
Predicates.instanceOf(ArrayList.class), which is inconsistent with equals."

What if you were to provide Collections2.filter(Collection unfiltered, Class 
type) and say this in its Javadoc:

"Warning: Predicates.instanceOf(type) must be consistent with equals, as 
documented at Predicate.apply. Do not provide a type such as ArrayList.class."

This would bring Collections2 up to feature parity with Iterables, regarding 
filtering.

If people want something that narrows the Collection type and throws an 
exception if the wrong type is encountered, by all means, add it too! What's 
the harm? I've just filed a new issue for that. Let's put any further 
discussion of it there:
http://code.google.com/p/guava-libraries/issues/detail?id=609

Thanks for your consideration.

Original comment by jaredjac...@gmail.com on 19 Apr 2011 at 5:52

GoogleCodeExporter commented 9 years ago
You might be thinking: "Why aren't you happy just using the existing 
Collections2.filter with Predicates.instanceof(Class)?"

Don't forget the cast and type warning. Here's the difference:

    // BEFORE
    @SuppressWarnings("unchecked")
    Collection<MarshallableManagedSlice> mms = 
        (Collection<MarshallableManagedSlice>) Collections2.filter(
            slices, Predicates.instanceOf(MarshallableManagedSlice.class));

    // AFTER
    Collection<MarshallableManagedSlice> mms = Collections2.filter(
        slices, MarshallableManagedSlice.class);

Original comment by jaredjac...@gmail.com on 21 Apr 2011 at 4:31

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 13 Jul 2011 at 7:34

GoogleCodeExporter commented 9 years ago

Original comment by fry@google.com on 10 Dec 2011 at 4:05

GoogleCodeExporter commented 9 years ago

Original comment by fry@google.com on 16 Feb 2012 at 7:17

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 30 May 2012 at 7:43

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 22 Jun 2012 at 6:16

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:15

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:09