AndersG
AndersG

Reputation: 365

Function with multiple return statement in if clauses does not behave as expected?

I'm not new to Java programming in any way, but this problem is unheard of to me. I have this code:

private static boolean isEarlierThanAndNotReminded(Callback left, Callback right) {    
if(right == null) {
            return !left.isReminded();
        }
        else {
            return !left.isReminded() && (left.getAlertStart().before(right.getAlertStart()));
 }
}

Ok so the problem is that I get a report of a null pointer on the line in the "else" clause. When I debug, I can see that right is actually null, but still the execution land first in the if-clause and then control continues into the "else"-clause where it gets a null pointer. I am clueless to what I am missing here, any suggestions?

Upvotes: 0

Views: 268

Answers (3)

NickT
NickT

Reputation: 23873

It's probably just a function of the optimisation of the compiler and the way the debugger handles it. It's going to the 'if' first condition but not actually executing the return. If you think about it, whatever the value of 'right' is the code needs to know the value of ' !left.isReminded()', so that statement has to be executed. I bet that if you started your function with

boolean leftRemindedValue = left.isReminded();

before the if statement and then did put that new boolean into your return statements, you would see a different execution path in the debugger.

Upvotes: 2

Blundell
Blundell

Reputation: 76536

Thats a bit of a mixed up If statement, I may not have your logic right but this should make more sense to you:

private static boolean isEarlierThanAndNotReminded(Callback left, Callback right) {    
     if(right != null) {
        return !right.isReminded();
     } else {
        return !left.isReminded();
     }

}

You want to check that right isn't null, if it isn't it means you have a 'right' callback so use the right object , else you assume you have a left callback and use this.

This bit of code:

  && (left.getAlertStart().before(right.getAlertStart())

needs you to check for both left and right null checks:

 if(left != null && right != null){
     return (left.getAlertStart().before(right.getAlertStart());
 }

So if I have this right you would end up with:

 private static boolean isEarlierThanAndNotReminded(Callback left, Callback right) {    
     boolean returnVal = false;
     if(right != null) {
        returnVal = !right.isReminded();
     } else if (left != null){
        returnVal = !left.isReminded();
     }
     if(left != null && right != null){
        returnVal  = returnVal && (left.getAlertStart().before(right.getAlertStart());
     }
     return returnVal;
}

Upvotes: 2

Xion
Xion

Reputation: 22770

The right is not the only reference which can be null in the else clause. The left.getAlertStart() might be null as well -- that's likely what causes the exception.

Upvotes: 1

Related Questions