kucci / guava-libraries

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

Loss of type safety with Itera*.filter(Itera*, Class) #455

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I had expected that this method, which should be used instead of the other 
overload with Predicates.instanceOf(Class), would raise a compilation error if 
I use incompatible classes. But instead, the returned Itera* will simply be 
empty.

Let's say we have two classes, Foo and Bar, which are not related. The 
following:

List<Foo> list = newArrayList();
list.add(new Foo());
Iterable<Bar> result = Iterables.filter(list, Bar.class);

will make result empty. While this behavior seems valid (nothing in the list 
passes the predicate), you are actually hiding a type cast error; I think it 
would be better to raise a compilation error since you are basically doing an 
instanceof of incompatible operand type. You can achieve this by changing the 
method signature from

public static <T> Iterable<T> filter( final Iterable<?> unfiltered, final 
Class<T> type)

to

public static <T> Iterable<T> filter( final Iterable<? super T> unfiltered, 
final Class<T> type)

I also think that this change would not need to keep the old signature and make 
it deprecated (anyways, you can't have both signature because of type erasure). 
Changing the method signature should keep binary compatibility since both 
methods have the same erasure and behavior doesn't change and the new signature 
will work with actual usage; that is, unless you get the new error and in this 
case you've found a bug in your own code.

Original issue reported on code.google.com by pascal.g...@gmail.com on 21 Oct 2010 at 6:13

GoogleCodeExporter commented 9 years ago
Hmmmmm... this is getting trickier... I guess I just found out why the old 
signature is used: if Bar is an interface instead of a class, then the 
signature I propose raises a compilation error even if the equivalent 
instanceof is valid. I guess this is the limit of generics...

The more I think and try to have it raise a compilation error but stay 
consistent with the equivalent instanceof expression, the more I find out that 
this signature is a bad idea (because interfaces are far more flexible in 
instanceof expression).

I was going to propose to add a dynamic type check using 
Class.isAssignableFrom() and throw an IllegalArgumentException, but then again 
there's a problem: you would have to retrieve an object from the iterable and 
test it. Since all those method are about views, that seems like it beats the 
purpose... AND it would be a dynamic check that fails at runtime instead of 
compile time like I wanted.

It bugs me that I can't have any pointer to this kind of error. It is all too 
easy to encounter by having two Iterable in the same scope and using the wrong 
one... If anyone has any ideas, please share.

Original comment by pascal.g...@gmail.com on 21 Oct 2010 at 8:28

GoogleCodeExporter commented 9 years ago
This code

List<Foo> list = newArrayList();
list.add(new Foo());
Iterable<Bar> result = Iterables.filter(list, Bar.class);

is perfectly valid when Foo and Bar are not provably incompatible types (e.g. 
String and Integer) -- for example if they are List and Serializable.  There is 
no way for us to express this in the signature.

Original comment by kevinb@google.com on 8 Dec 2010 at 3:38

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 3 Nov 2014 at 9:09