Why does this assignment cause NPE?

Question

public class Npe {
    static class Thing {
        long value;
    }

    public static Map<Thing, Long> map;

    public static void main(String[] args) {
        Thing thing = new Thing();
        method(null); // returns -1
        method(thing); // returns 0
        map = new HashMap<Thing, Long>();
        method(null); // returns -1
        method(thing); // NullPointerException thrown inside this method call
    }

    public static long method(Thing thing) {
        if (thing == null) {
            return -1;
        }
        Long v = (map == null) ? thing.value : map.get(thing); // NPE here
        if (v == null) {
            v = thing.value;
        }
        return v;
    }
}

On the 4th call to method() I get a NullPointerException thrown on the indicated line inside method(). If I refactor that line from

Long v = (map == null) ? thing.value : map.get(thing);

to

Long v;
if (map == null) {
    v = thing.value;
} else {
    v = map.get(thing);
}

I get no NullPointerException and the method behaves as it should. The question is: WHY??

It looks to me like the compiler expects the result of the ? operator to be long so that it is automatically unboxing (demoting from Long to long) the result of the call to map.get(thing) (which may return null and therefore throw a NullPointerException). IMHO it should be expecting the result of the ? operator to be Long and autoboxing (promoting long to Long) thing.value instead.

Even better, if I refactor this statement:

Long v = (map == null) ? thing.value : map.get(thing);

to this (casting long to Long explicitly):

Long v = (map == null) ? (Long)thing.value : map.get(thing);

my IDE (IntelliJ) says that the cast is redundant, but the compiled code works as expected and does not throw a NullPointerException! :-D

possible duplicate of [Booleans, conditional operators and autoboxing](http://stackoverflow.com/questions/3882095/booleans-conditional-operators-and-autoboxing) — Bhesh Gurung, Apr 10 '14 at 15:39
@DwB Your comment is not relevant as I am not a junior programmer. And your suggestion to "not use the ternary operator" is silly as well. Just because something may be complicated or confusing that doesn't mean you shouldn't use it. It just means that you need to be careful and know what you are doing. If developers never used anything that was complicated or potentially confusing, they would all be painters or street sweepers or gardeners instead of programmers ;-) — David Wasser, Apr 10 '14 at 15:50
If you dont' understand a ternary operator, then, believe it or not, you are still junior with java. Also, the fact that something is complicated, confusing, and provides no value is a great reason to not use it. If developers never used anything that was complicated and potentially confusing, they would produce code that is less unnecessairly complicated and confusing. — DwB, Apr 10 '14 at 16:07

Rohit Jain · Accepted Answer · 2014-04-10T15:43:44.613

29

Consider your conditional expression:

(map == null) ? thing.value : map.get(thing)

The result of that expression will be long, because the type of thing.value is long. See JLS §15.25 - Conditional Operator. The table in JLS 8 is a great addition. It clarifies all the possible output types for different input types. So much was the confusion related to the type of conditional expressions.

Now when you invoke this method as:

method(thing);

The map is not null, so the condition map == null in your expression evaluates to false, and then it evaluates map.get(thing) to get the result.

Since there is no entry in map yet, map.get(thing) will return null. But since the type of result is long, an unboxing operation is performed on null, which results in NPE.

Now when you explicitly cast thing.value to Long, the type of expression becomes Long. So no unboxing is performed on the result of map.get(thing), and null is assigned to Long v.

edited Apr 10 '14 at 15:43

answered Apr 10 '14 at 15:36

Rohit Jain

209,639
45
409
525

4

*Common* Java gotcha :) – Jean Logeart Apr 10 '14 at 15:37
Why do you say that the result of the expression will be `long`? I would say that the result of the expression is `Long`, because the 3rd argument is `Long`. – David Wasser Apr 10 '14 at 15:44
1

@DavidWasser The result of conditional expression depends on the type of both 2nd and 3rd operand. Not just on the operand that is going to be the answer. See the table in the JLS link. If any of the operand is primitive type, the result will be primitive type. – Rohit Jain Apr 10 '14 at 15:46
Thanks for the link. The table is interesting and enlightening. I still think it is interesting that even my IDE didn't recognize that the explicit cast of `long` to `Long` was, in reality, NOT redundant at all! – David Wasser Apr 10 '14 at 15:54
2

However I still maintain that this is a Java bug. There is no reason to **demote** the reference argument to match the other (primitive) argument instead of **promoting** the primitive argument to match the other (reference) argument. That's just stupid IMHO. This is likely to cause NPE in cases where it isn't necessary and has no benefits that I can see over the alternative. – David Wasser Apr 10 '14 at 16:09
4

@DavidWasser Firstly, the behaviour that is clearly listed in JLS, should never count as bug. Any operator has to be implemented one way or the other. It's completely a design decision, and better be left with Java designers as to how they implement a particular operator. – Rohit Jain Apr 10 '14 at 16:11
@DavidWasser: There are at least two benefits. (1) A preference for primitives vastly simplifies conversions. Objects live within a much more rigid type system than primitives do; convertibility (aside from autoboxing) is entirely constrained by inheritance. You can't just up and convert an `Integer` to a `Long`, for example, because neither is a superclass of the other. Instead, you'd get the primitive value as a `long`, and then box that as a `Long`. Notice, *you're already unboxing.* Where's the benefit over unboxing `123` instead and just stopping there? Particularly considering... – cHao Apr 11 '14 at 03:20
@DavidWasser: (2) Consistent demotion makes operators (particularly `==`) less boneheaded. Consider `Integer a = 123456; System.out.println(a == 123456);`. Were promotion the default, that would print `false` -- the literal would be boxed, and both objects would be tested for *reference* equality (ie: that they're the exact same object, which they typically won't be). Whereas with demotion as the default, the two sides are tested for *value* equality, and you thus get the far more reasonable `true`. – cHao Apr 11 '14 at 03:21

score 0 · Answer 2 · answered Apr 10 '14 at 15:57

0

This is my understanding of what is happening :

when you use Long v = (map == null) ? thing.value : map.get(thing); // NPE here

the map.get(thing) returns a Long that is null and then there is an attempt to unbox its value to long ( because the expression type if long ) - this causes NPE.

However when you use the long form , you are carefully avoiding the unboxing operation on the null Long.

answered Apr 10 '14 at 15:57

Bhaskar

7,443
5
39
51

ok I am using a version of IE that does not believe in showing SO updates as quickly as they happen, and not within 22 mins atleast :) – Bhaskar Apr 10 '14 at 16:00

Why does this assignment cause NPE?

2 Answers2