NPE throwing behavior of immutable collections

[John Hendrikx]

TLDR; why does contains(null) not just return false for collections that 
don't allow nulls. Just because the interface allows it, does not mean 
it should do it as it devalues the usefulness of the abstraction 
provided by the interface.

Background:

I'm someone that likes to check correctness of any constructor or method 
parameter before allowing an object to be constructed or to be modified, 
in order to maintain invariants that are provided by the class to its users.

This ranges from simple null checks, to range checks on numeric values, 
to complete checks like "is a collection sorted" or is a list of nodes 
acyclic. Anything I can check in the constructor that may avoid problems 
further down the line or that may affect what I can guarantee on its own 
API methods.  For example, if I check in the constructor that something 
is not null, then the associated getter will guarantee that the returned 
value is not null.  If I check that a List doesn't contain nulls, then 
the associated getter will reflect that as well (assuming it is 
immutable or defensivily copied).

For collections, this is currently becoming harder and harder because 
more and more new collections are written to be null hostile.  It is 
fine if a collection doesn't accept nulls, but throwing NPE when I ask 
such a collection if it contains null seems to be counter productive, 
and reduces the usefulness of the collection interfaces.

This strict interpretation makes the collection interfaces harder to use 
than necessary. Interfaces are only useful when their contract is well 
defined. The more things an interface allows or leaves unspecified, the 
less useful it is for its users.

I know that the collection interfaces allow this, but you have to ask 
yourself this important question: how useful is an interface that makes 
almost no guarantees about what its methods will do? Interfaces like 
`List`, `Map` and `Set` are passed as method parameters a lot, and to 
make these useful, implementations of these interfaces should do their 
best to provide as consistent an experience as is reasonably possible.  
The alternative is that these interfaces will slowly decline in 
usefulness as methods will start asking for `ArrayList` instead of 
`List` to avoid having to deal with a too lenient specification.

With the collection interfaces I get the impression that recently there 
has been too much focus on what would be easy for the collection 
implementation instead of what would be easy for the users of said 
interfaces. In my opinion, the concerns of the user of interfaces almost 
always far outweigh the concerns of the implementors of said interfaces.

In the past, immutable collections were rare, but they get are getting 
more and more common all the time.  For example, in unit testing. 
Unfortunately, these immutable collections differ quite a bit from their 
mutable counterparts.  Some parts are only midly annoying (not accepting 
`null` as the **value** in a `Map` for example), but other parts require 
code to be rewritten for it to be able to work as a drop-in replacement 
for the mutable variant. A simple example is this:

     public void addShoppingItems(Collection<String> shoppingItems) {
         if (shoppingItems.contains(null)) {  // this looks quite 
reasonable and safe...
             throw new IllegalArgumentException("shoppingItems should 
not contain nulls");
         }

         this.shoppingItems.addAll(shoppingItems);
     }

Testing this code is annoying:

      x.addShoppingItems(List.of("a", null"));   // can't construct 
immutable collection with null

      x.addShoppingItems(Arrays.asList("a", null"));  // fine, go back 
to what we did before then...

The above problems, I suppose we can live with it; immutable collections 
don't want `null` although I don't see any reason to not allow it as I 
can write a similar `List.of` that returns immutable collections that do 
allow `null`. For JDK code this is a bit disconcerting, as it is 
supposed to be as flexible as is reasonable without having too much of 
an opinion about what is good or bad.

This next one however:

      assertNoExceptionThrown(() -> x.addShoppingItems(List.of("a", 
"b")));  // not allowed, contains(null) in application code throws NPE

This is much more insidious; the `contains(null)` check has been a very 
practical way to check if collections contain null, and this works for 
almost all collections in common use, so there is no problem.  But now, 
more and more collections are starting to just throw NPE immediately 
even just to **check** if a null element is present. This only serves to 
distinguish themselves loudly from other similar collections that will 
simply return `false`.

I think this behavior is detrimental to the java collection interfaces 
in general, especially since there is a clear answer to the question if 
such a collection contains null or not. In fact, why `contains` was ever 
allowed to throw an exception aside from "UnsupportedOperationException" 
is a mystery to me, and throwing one when one could just as easily 
return the correct and expected answer seems very opiniated and almost 
malicious -- not behavior I'd expect from JDK core libraries.

Also note that there is no way to know in advance if `contains(null)` is 
going to be throwing the NPE. If interfaces had a method `allowsNulls()` 
that would already be an improvement.

--John