Ternary operator evaluating conditional statement while condition not met

Question

I have written some code; here the relevant snippets:

@NonNullByDefault
public class Score<NUMERAL, LITERAL>
{
    protected NUMERAL value;
    @Nullable
    protected LITERAL literal;
    [...]

I have overwritten my equals()method as follows:

@Override
public boolean equals(@Nullable Object object)
{
    if(object == null) return false;
    if(object == this) return true;

    if( object instanceof Score)
    {
        return ((Score<NUMERAL, LITERAL>) object).getValue().equals(value) &&
                literal == null ? ((Score<NUMERAL, LITERAL>) object).getLiteral() == null : literal.equals(((Score<NUMERAL, LITERAL>) object).getLiteral());
    }
    return false;
}

Basically, the idea is that a Score may only have a numeric value in which case the literal is null. I have written some unit tests and get a null pointer exception with the code below:

[....]
Score<Float, String> score = new Score<>(0.0f);
Score<Float, String> anotherScore = new Score<>(1.0f, "One");
[....]

assertFalse(score.equals(anotherScore));

If I am not mistaken, shouldn't short-cutting in equals prevent anything after the && from being executed as the first expression is already false? Furthermore, why the exception? As the conditional is true, I would expect the expression of the ternary to be evaluated and the conditional expression skipped. From what I have read in the specifications, this should be the behaviour. Furthermore, I found this question: Java ternary (immediate if) evaluation which should lend some more leverage to my thought process.

Maybe I have overlooked something rather obvious but I am out of ideas. Maybe you can help?

Answer 1

It short-circuits alright, but not quite the way you want it to. && has a higher precedence than the ternary ?: - therefore this (indentation, line breaks and comments added to clarify)

((Score<NUMERAL, LITERAL>) object).getValue().equals(value) &&
literal == null
    ? ((Score<NUMERAL, LITERAL>) object).getLiteral() == null
    : literal.equals(((Score<NUMERAL, LITERAL>) object).getLiteral())

actually means this:

//the first line as a whole is the condition for ?:
((Score<NUMERAL, LITERAL>) object).getValue().equals(value) && literal == null
    ? ((Score<NUMERAL, LITERAL>) object).getLiteral() == null
    : literal.equals(((Score<NUMERAL, LITERAL>) object).getLiteral())

This means, in practice, that if the first part of the condition is false but literal is null, you automatically enter the : part of the expression where you call literal.equals, causing the NullPointerException.

The fix is simple: add parentheses to tell Java which way you want stuff to be evaluated:

((Score<NUMERAL, LITERAL>) object).getValue().equals(value) && 
(literal == null 
    ? ((Score<NUMERAL, LITERAL>) object).getLiteral() == null
    : literal.equals(((Score<NUMERAL, LITERAL>) object).getLiteral()))