NPE on java stream reduce operation

Question

Recently, while working with Java 8 streams, I came across a NullPointerException on a reduce operation while working with the following test cases:

private static final BinaryOperator<Integer> sum = (a, b) -> {
    if (a == null) return b;
    if (b == null) return a;
    return Integer.sum(a, b);
};

List<Integer> s = new ArrayList<>();
s.add(null);
s.add(null);
s.add(null);

Integer i = s.stream().reduce(sum).orElse(null);
// throws NPE

Integer i = s.stream().reduce(sum).orElse(2);
// throws NPE

Integer i = s.stream().reduce(null,(a, b)->null);
// returns a value i.e null

Or alternatively:

Integer i = s.stream().filter(Objects::nonNull).reduce(Integer::sum).orElse(null);
// returns a value i.e null

Upon checking the reduce operation, I came across this class which performs the reduce operation:

class ReducingSink implements AccumulatingSink<T, Optional<T>, ReducingSink> {
    private boolean empty;
    private T state;

    public void begin(long size) {
        empty = true;
        state = null;
    }

    @Override
    public void accept(T t) {
        if (empty) {
            empty = false;
            state = t;
        } else {
            state = operator.apply(state, t);
        }
    }

    @Override
    public Optional<T> get() {
        return empty ? Optional.empty() : Optional.of(state);
    }

    @Override
    public void combine(ReducingSink other) {
        if (!other.empty)
            accept(other.state);
    }
}

In the above code, you see that the get() method returns an optional value if the boolean empty is false, and in my case the value is false but state is null, so Optional.of(null) throws a NullPointerException. In my case I have a binary operator which allows null.

So I think the code

return empty ? Optional.empty() : Optional.of(state);

should be changed to

return empty || state == null ? Optional.empty() : Optional.of(state);

As my binary operator (which has the task of reducing) and is okay with null.

Answer 1

I can't really tell why you have to work with nulls, this seems like a bad a idea to begin with. And, as you have seen you can't reduce using a null as input. You could build your own custom Collector (you can't build your own Reducer).

What you have in place:

 Double result = s.stream()
         .filter(Objects::nonNull)
         .reduce(Double::sum)
         .orElse(null);

is perfectly fine btw. The only way to get a null result is when all elements from your input are null, thus filtering them initially is the way to go. For the fun of it I decided to write a custom collector (can't really tell why, thought it would be fun I guess)

 Double result = s.stream()
          .parallel()
          .collect(
                () -> new Double[] { null }, 
                (left, right) -> {
                    if (right != null) {
                       if (left[0] != null) {
                           left[0] = right + left[0];
                       } else {
                            left[0] = right;
                       }
                    }
                }, 
                (left, right) -> {
                   if (right[0] != null) {
                        if (left[0] != null) {
                            left[0] = right[0] + left[0];
                        } else {
                             left[0] = right[0];
                        }
                }})[0];

You could put this into a class itself if needed:

 class NullableCollector implements Collector<Double, Double[], Double> {

    @Override
    public BiConsumer<Double[], Double> accumulator() {
        return (left, right) -> {
            if (right != null) {
                if (left[0] != null) {
                    left[0] = right + left[0];
                } else {
                    left[0] = right;
                }
            }
        };
    }

    @Override
    public Set<Characteristics> characteristics() {
        return EnumSet.noneOf(Characteristics.class);
    }

    @Override
    public BinaryOperator<Double[]> combiner() {
        return (left, right) -> {
            if (right[0] != null) {
                if (left[0] != null) {
                    left[0] = right[0] + left[0];
                } else {
                    left[0] = right[0];
                }
            }
            return left;
        };
    }

    @Override
    public Function<Double[], Double> finisher() {
        return (array) -> array[0];
    }

    @Override
    public Supplier<Double[]> supplier() {
        return () -> new Double[] { null };
    }

}

Answer 2

The documentation of the reduce operation that you use states:

Throws: NullPointerException - if the result of the reduction is null

So the NPE that you see is documented and intended outcome even if your binary operator is fine with null.

The documentation is even more verbose giving extra insight with some equivalent code:

     boolean foundAny = false;
     T result = null;
     for (T element : this stream) {
         if (!foundAny) {
             foundAny = true;
             result = element;
         }
         else
             result = accumulator.apply(result, element);
     }
     return foundAny ? Optional.of(result) : Optional.empty();

NPE is thrown on the last line in this case.

If your suggested change is applied in the library we won't be able to distinguish the result of reducing an empty stream from a stream in which the result of the reduction is null.