NPE throwing behavior of immutable collections

[Glavo]

I quite agree with you. I think the collection framework is in a very
puzzling state.

The hostility of the new collection factory method introduced by Java 9 to
null has brought us trouble.
I can understand that this is to expect users to explicitly declare that
the elements of the list cannot be null, rather than in an ambiguous state.
However, this makes migration more difficult, and the cost of scanning all
elements to ensure that they are not null is also unpleasant.
I hope to provide an additional set of factory methods for the collection
to accept nullable elements.

In addition, it is also confusing to share the same interface between
mutable collections and immutable collections .
Before encountering UnsupportedOperationException, we can't even know what
operations a collection supports.
This problem has existed for a long time, but no solution has been given.


On Sun, Jan 29, 2023 at 11:28 PM John Hendrikx <john.hendrikx at gmail.com>
wrote:

> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20230130/750e5410/attachment.htm>