Strictly speaking such an implementation would be wrong.
The reason for this is that even if an object is not of type E
, it could still return true
on an equals()
call.
Assume for a second, that you've got a class like this:
public class FakeString {
private final String value;
public FakeString(String value) {
if (value == null) {
throw new IllegalArgumentException();
}
this.value = value;
}
public int hashCode() {
return value.hashCode();
}
public boolean equals(Object o) {
return value.equals(o);
}
}
Then this code would print true
:
List<String> strings = Arrays.asList("foo", "bar", "baz");
System.out.println(strings.contains(new FakeString("bar")));
And just to clarify: this behaviour is intended and is the reason why contains()
takes an Object
instead of E
. The same is true for remove()
, by the way.
Your int
will be boxed to an Object
.
(int) === boxing ===> (Integer) ==== reference widening ===> (Object)
But Netbeans is smart and detects this, notices that your Integer
is incompatible with a String
. Normally this would throw a ClassCastException
. However, the javadoc clearly states that throwing the ClassCastException on Collections.contains(Object) is optional.
So in this case, there would be no benefit to overload the contains
method to allow an Element
type because int
, float
, double
, etc can be autoboxed to Object
successfully. Therefore, a contains
with just Object
is just fine.
For reasons unknown, List
, HashSet
, Set
, and LinkedHashSet
do not throw a ClassCastException
if "f the type of the specified element is incompatible with this [insert type here] (optional)". Note the optional part. As you have shown, Netbeans will give you a suspicious warning when you try to pass an incompatible type. The trap here is if you try to use logic to rely on the return value of .contains(...)
. A developer who doesn't pay attention to his warnings could fall into the trap of assuming that contains
will always work, but then it doesn't.
As the developer at this blog recommends, here are four steps to avoid this pitfall in the future:
Careful coding and code reviews might lead to developers or reviewers seeing that an object of an irrelevant class is being passed
to the contains method. I'm typically uncomfortable leaving it with
just this form of protection.
Use of a modern version of NetBeans or of similar tools that flag the "suspicious" behavior can be very helpful.
Unit tests combined with code coverage tests can be helpful in identifying seemingly strange flows through the code that can be
attributed to issues such as the one described in this post.
Collection implementations that do throw ClassCastException at least provide a runtime error to let developers know that an improper
action is being taken. It may not be as effective as compile-time
detection, but it does make it much easier to identify that there is a
problem and what it is when it happens then without it. However, the
major disadvantage with this approach is that the choice of which
collection implementation to use is often driven by important
considerations that often outweigh the desire to have runtime
detection of an errant call to contains.