Take a look at the following piece of code:
package test;
import java.util.Date;
public class Test {
public static String format(Date date) {
return String.valueOf(date);
}
public static String format(long millis) {
return format(new Date(millis));
}
public static String format(Instant instant) {
return format(instant == null ? null : instant.getMillis());
}
public static class Instant {
private long millis;
public Instant(long millis) {
this.millis = millis;
}
public long getMillis() {
return millis;
}
}
public static void main(String[] args) {
Instant instant = null;
format(instant);
}
}
Any idea what the output will be? Turns out this throws a
NullPointerException:
Exception in thread "main" java.lang.NullPointerException
at test.Test.format(Test.java:16)
at test.Test.main(Test.java:34)
Interesting. Line 16 is clearly fishy: The idea is that if the
instant is null, the
format(Date) method should be called, while
format(long) should be called otherwise. It turns out things are much more contrived at the bytecode level (use
javap -c):
public static java.lang.String format(test.Test$Instant);
Code:
0: aload_0
1: ifnonnull 8
4: aconst_null
5: goto 15
8: aload_0
9: invokevirtual #35; //Method test/Test$Instant.getMillis:()J
12: invokestatic #41; //Method java/lang/Long.valueOf:(J)Ljava/lang/Long;
15: invokevirtual #46; //Method java/lang/Long.longValue:()J
18: invokestatic #49; //Method format:(J)Ljava/lang/String;
21: areturn
This actually translates to the following Java code, which clearly explains the
NullPointerException:
public static String format(Instant instant) {
return format((instant != null ? Long.valueOf(instant.getMillis()) : null).longValue());
}
Sometimes autoboxing can really rear its ugly head!
That's a nasty NPE!
ReplyDeleteI saw in some circles that autoboxing has a very bad reputation, but I think that is overstated. In fact I think the conditional ? operator is even worst for the type system than autoboxing because from the 2nd and 3er operands it tries to come up with a *single* type...and that's a ticking bomb.
IMO, the main bug here is using the conditional ? operator inside an overloaded method call. Of course, autoboxing make things worst! :P
Change that method call with a good old if-then-else and the problem is gone.
See my posts on the subject:
http://www.hexaid.com/blog/2010/09/12/some-confusing-cases-of-method-overloading/
http://www.hexaid.com/blog/2010/09/14/some-confusing-cases-of-method-overloading-part-2/
http://www.hexaid.com/blog/2010/09/19/some-confusing-cases-of-method-overloading-%e2%80%93-part-3/
Gabriel
When I coded a similar piece of code earlier this week I was even surprised that the compiler didn't complain, thinking "How can this possibly work, I'm calling two methods with a single method call!?!". Of course it didn't work, so I indeed replaced it with a good old if-then-else.
ReplyDeleteI completely agree about the sleekness of this example, but I also agree with Gabriel here.
ReplyDeleteThe problem is a misunderstanding of the conditional operator.
"The idea is that if the instant is null, the format(Date) method should be called, while format(long) should be called otherwise."
By making this assumption you forgot that the conditional operator can only return a single type.
So it would return Date or long, but not both of them.
(and its clear that if one of types is primitive long he could never choose Date as the common type).
That is when the boxing and casting story comes in to play (boxes long to Long and casts null to Long according to the promotion rules).
About why the compiler doesn't generate an error is probably the same reason as why (at least by default) there is no error detected in doing this neither:
public static void test(long l) {
}
public static void main(String args[]) {
test((Long) null);
}
The main point here is not why it compiles or why its generates a NPE or whether I'm misunderstanding the conditional operator or anything along those lines.
ReplyDeleteThe main point actually is that autoboxing can violate the principle of least astonishment.