Inconsistent handling of contains(null) for collections.

[GoogleCodeExporter]

As far as I know none of the Collection implementations in Guava permit nulls.  
They will throw a NullPointerException if you try to call add(null), put(null, 
X) etc. on them.

However most of them simply return false rather than throwing NPE if you call 
contains(null) on them. (If I understand correctly the Collection interface 
allows either behaviour.)

The maps created by MapMaker behave differently.  Calling contains(null) on 
their keySet or values collections throws a NPE.  This also implies that if you 
derive a Set from one of these maps (using Sets.newSetFromMap) then that Set 
will also throw NPE.

Wouldn't it be better to make all Guava collections consistent with each other 
rather than making just one of them (that I know of - CustomConcurrentHashMap) 
consistent with java.util.ConcurrentHashMap?

Related to this StackOverflow question:
http://stackoverflow.com/questions/3141149/java-list-contains-question

Original issue reported on code.google.com by fin...@gmail.com on 13 Jul 2010 at 11:56

[GoogleCodeExporter]

I tend to side with the notion that contains(null) should return false, but 
others believe (sometimes strongly) that for a collection that doesn't allow 
nulls, returning false would just mask a probable bug.  So we never embarked on 
making all our collections consistent because we couldn't agree which direction 
to go.  We also figured at least the JDK is just as inconsistent as we are on 
this, if not more so.

I find the inconsistency ... tolerable.

Then the same problem exists with ClassCastExceptions (typically only in 
SortedSets and SortedMaps, since Comparator.compare() and 
Comparable.compareTo() are spec'd to throw that, unlike Object.equals().  I 
have sometimes used the terms "Type A" and "Type B" collections, like the 
well-known personality types.  A Type B collection is chill, telling you "no, I 
don't have that," while a Type A collection is like "HOW DARE YOU EVEN ASK ME 
THAT!"

(BTW, contrary to your opening statement, our general-purpose collections like 
HashBiMap, ArrayListMultimap and so forth do permit null.  Immutable and 
concurrent ones don't.  This is reasonably consistent with the JDK.)

Original comment by kevinb@google.com on 13 Jul 2010 at 8:59

[GoogleCodeExporter]

Regarding contains(null): I agree that it should return false. I think the 
notion of throwing NPE is too anal. The contract for Object.equals() specifies 
that false should be returned if null is passed in, and I think a proper 
abstraction of Collection.contains(o) is: return element1.equals(o) || 
element2.equals(o) || ...

However, I would love to see the Immutable collections support null.

Original comment by ray.j.gr...@gmail.com on 11 Aug 2010 at 11:20

[GoogleCodeExporter]

Original comment by fry@google.com on 28 Jul 2011 at 5:21

• Changed state: Accepted
• Added labels: Type-Enhancement

[GoogleCodeExporter]

We actually added tests for this a month or two ago and found that everything 
already passes.

Original comment by kevinb@google.com on 5 Oct 2011 at 5:10

• Changed state: Fixed
• Added labels: Milestone-Release10

[GoogleCodeExporter]

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

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

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

• Added labels: Restrict-AddIssueComment-Commit