I tend to agree.
Presumably it is similar reasoning that led to Collection<E>.contains() to take
Object rather than E.
> On Jan 29, 2023, at 7:28 AM, John Hendrikx <john.hendr...@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
>
>
