How to determine whether a Java collection contains `null`

user_4142130 0
Viewed 613

I want to write a method which takes a collection as a parameter. However, the collection cannot contain null. If it contains null, this will lead to a NullPointerException later on, when it is not clear anymore where the null came from. Therefore, I want to validate the input. The naive approach would be:

public void doSomething(Collection<?> collection) {
    if (collection.contains(null))
        throw new IllegalArgumentException("collection cannot contain null!");

    // do something
}

However, Collection.contains may throw a NullPointerException if the collection does not support null values. In fact, Java's own immutable collections do that. So, in the naive approach, the following code does not work:

doSomething(List.of("a", "b", "c")); // NullPointerException!

There are a few possible solutions, but none of them feel right.

For example, I could search for null myself:

public void doSomething(Collection<?> collection) {
    for (var e : collection) {
        if (e == null)
            throw new IllegalArgumentException("collection cannot contain null!");
    }
    // do something
}

But now I'm doing a linear search, although the collection might be a HashSet or a TreeSet, offering much better performance.

Or I could catch the NullPointerException:

public void doSomething(Collection<?> collection) {
    try {
        if (collection.contains(null))
            throw new IllegalArgumentException("collection cannot contain null!");
    } catch (NullPointerException e) {
        // Apparently, collection doesn't support null. This is good!
    }
    // do something
}

But this feels very unsafe. Can I really be sure that that NullPointerException was caused by the collection not supporting nulls, or did I just eat an Exception which could provide valuable information on a bug?

Is there a good way to determine whether a collection contains null?

collections java
2 Answers

I don’t think, replacing a NullPointerException with an IllegalArgumentException is an improvement. It requires us to read a message to get an information the dedicated exception type tells us intrinsically.

Of course, checking incoming arguments as early as possible, to avoid detaching the source of the problem from the place where the exception occurs, is a good thing. When I use, e.g.

public static void main(String[] args) {
    new NullCheckExample().doSomething(Arrays.asList("foo", "bar", null));
}

public void doSomething(Collection<?> collection) {
    collection.forEach(Objects::requireNonNull);

    // do something
}

I get

Exception in thread "main" java.lang.NullPointerException
    at java.base/java.util.Objects.requireNonNull(Objects.java:208)
    at java.base/java.util.Arrays$ArrayList.forEach(Arrays.java:4204)
    at NullCheckExample.doSomething(NullCheckExample.java:12)
    at NullCheckExample.main(NullCheckExample.java:8)

which I consider it expressive enough, to help tracing the cause.

Now, is there a way to avoid the costs of a linear search when null is impossible? In theory, yes. We can do

public void doSomething(Collection<?> collection) {
    Spliterator<?> sp = collection.spliterator();
    if(sp.hasCharacteristics(Spliterator.NONNULL)) {
        // yeah, null is impossible
    }
    else if(sp.hasCharacteristics(Spliterator.SORTED) && sp.getComparator() == null) {
        // natural order precludes null too
    }
    else {
        // here we have to check,
        // collection.forEach(Objects::requireNonNull)
        // or 
        // if(collection.contains(null)) throw ...
    }

    // do something
}

which does work for some collections, e.g. the concurrent collections or a TreeSet. The problem is, it doesn’t work for List.of(…) or Set.of(…) kind of collections. In the reference implementation (OpenJDK), the spliterator method has not been overridden, which not only means that Stream operations are less efficient, we also do not get NONNULL nor IMMUTABLE characteristics.

So in these cases, we end up doing an unnecessary linear search or have to deal with contains(null) throwing a NullPointerException (in the common case, rather than the erroneous case).

Depending on what you want to do, you might do something like

public void doSomething(Collection<?> collection) {
    List<?> workingCopy = List.copyOf(collection);

    // do something
}

This bears a copy operation, but does protect you against subsequent modifications of the collection, which might be needed anyway, to ensure consistency. The working copy is guaranteed to be free of null and not to change. If the caller passes the result of List.of(…) into this method, which fulfills the criteria already, the copyOf will become a no-op, just returning the argument. So this is the best option when you expect the input to be an immutable list already most of the times.

But if you’re going to say that neither is a satisfying solution, I’d agree. There should be efficient possibilities to preclude null. Rejecting attempts to query for null is fine, but it should not end up in having no way to efficiently ensure that there is no null. It’s also baffling that there are no efficient spliterator() nor forEach implementations for the immutable collections, despite their implementations would be very simple, one-liners in most cases.

user_27114880

If your Java version is >= 8, you can use the stream API:

List<YourObject> newList = userListCollection.stream().filter(Objects::nonNull).collect(Collectors.toList);

I think, the stream() method is optimised for each collection type, so you shouldn't face performance issues. However, you still have to check if the whole collection is null.

user_71327390
Related