12

When I write my java code like this:

Map<String, Long> map = new HashMap<>()
Long number =null;
if(map == null)
    number = (long) 0;
else
    number = map.get("non-existent key");

the app runs as expected but when I do this:

Map<String, Long> map = new HashMap<>();
Long number= (map == null) ? (long)0 : map.get("non-existent key");

I get a NullPointerException on the second line. The debug pointer jumps from the second line to this method in the java.lang.Thread class:

 /**
     * Dispatch an uncaught exception to the handler. This method is
     * intended to be called only by the JVM.
     */
     private void dispatchUncaughtException(Throwable e) {
         getUncaughtExceptionHandler().uncaughtException(this, e);
     }

What is happening here? Both these code paths are exactly equivalent isn't it?


Edit

I am using Java 1.7 U25

jason
  • 236,483
  • 35
  • 423
  • 525
radiantRazor
  • 477
  • 3
  • 13
  • Why are you testing for the impossible? You create the `map` yourself; if it is `null`, your runtime system is broken. Since this is obviously a shortened version of other work you are doing, I would suggest you ensure that objects are always initializedso you don't have to do the `map == null` checks. The keys are your problem. – Eric Jablow Jul 09 '13 at 13:57
  • @Eric Jablow This is just a simplified version of my code. The map in question is actually being retrieved from another place where they are dynamically created on demand. That is why the check. Here I included the initialization so that it is clear to people who are answering that the if-true case is not being executed when my problem is occuring – radiantRazor Jul 09 '13 at 14:02
  • I now understand. Probably the best thing would be to throw an exception immediately if someone were to pass a `null` map to your code. Or, if someone passes in a `null` to your code, set your map to an empty one instead. – Eric Jablow Jul 09 '13 at 18:19
  • 1
    possible duplicate of [strange Java NullPointerException with autoboxing](http://stackoverflow.com/questions/3265948/strange-java-nullpointerexception-with-autoboxing) – default locale Jul 12 '13 at 11:15
  • 2
    Actually, there is [an older and more popular similar question](http://stackoverflow.com/questions/3882095/booleans-conditional-operators-and-autoboxing) – default locale Jul 12 '13 at 11:26

3 Answers3

15

They are not equivalent.

The type of this expression

(map == null) ? (long)0 : map.get("non-existent key");

is long because the true result has type long.

The reason this expression is of type long is from section §15.25 of the JLS:

If one of the second and third operands is of primitive type T, and the type of the other is the result of applying boxing conversion (§5.1.7) to T, then the type of the conditional expression is T.

When you lookup a non-existant key the map returns null. So, Java is attempting to unbox it to a long. But it's null. So it can't and you get a NullPointerException. You can fix this by saying:

Long number = (map == null) ? (Long)0L : map.get("non-existent key");

and then you'll be okay.

However, here,

if(map == null)
    number = (long) 0;
else
    number = map.get("non-existent key");

since number is declared as Long, that unboxing to a long never occurs.

jason
  • 236,483
  • 35
  • 423
  • 525
  • In that case, could the poster use the ternary operator by casting 0 to `Long` instead of `long`? – ameed Jul 09 '13 at 13:48
  • 1
    Yes, he'd have to cast `0L` to a `Long` though. – jason Jul 09 '13 at 13:52
  • 2
    Woww, the JLS has so many hidden catches. My obvious follow up question would be why is the JLS defined to infer the type as the primitive one? If it was defined that the type should be inferred as the autoboxed type, then such nullpointer exceptions could have been avoided. I will try asking that as a separate question maybe – radiantRazor Jul 09 '13 at 14:04
3

What is happening here? Both these code paths are exactly equivalent isn't it?

They are not equivalent; the ternary operator has a few caveats.

The if-true argument of the ternary operator, (long) 0, is of the primitive type long. Consequently, the if-false argument will be automatically unboxed from Long to long (as per JLS §15.25):

If one of the second and third operands is of primitive type T, and the type of the other is the result of applying boxing conversion (§5.1.7) to T, then the type of the conditional expression is T.

However, this argument is null (since your map does not contain the string "non-existent key", meaning get() returns null), so a NullPointerException occurs during the unboxing process.

arshajii
  • 127,459
  • 24
  • 238
  • 287
0

I commented above suggesting that he ensure map never be null, but that does not help with the ternary problem. As a practical matter, it's easier to let the system do the work for you. He could use Apache Commons Collections 4 and its DefaultedMap class.

import static org.apache.commons.collections4.map.DefaultedMap.defaultedMap;

Map<String, Long> map = ...;  // Ensure not null.
Map<String, Long> dMap = defaultedMap(map, 0L); 

Google Guava doesn't have anything as easy as that, but one can wrap map with the Maps.transformValues() method.

Eric Jablow
  • 7,874
  • 2
  • 22
  • 29