Unnecessary 'else' statement

Question

As you know, in Eclipse you can turn on "Unnecessary 'else' statement" check that will trigger on if-then-else with premature return. And, from my experience, there are two most possible situations when use such statement:

1) Pre-check:

if (!validate(arg1)) {
    return false;
}
doLotOfStuff();

2) Post-check:

doLotOfStuff();
if (condition) { 
    return foo; 
} else {
    return bar; 
}

In the second case, if the trigger is on, Eclipse will suggest you to change the code to:

doLotOfStuff();
if (condition) { 
    return foo; 
} 
return bar; 

However, I think that the return with else statement is more readable as it is like direct mapping of business logic. So I am curios if this "Unnecessary 'else' statement" code convention is widespread or code with else statement is more preferable?

Answer 1

Generally I would prefer the structure of the code to follow the structure of the underlying "business" logic. In this case, my approach would depend what condition represents. If it is an error check, for example, which won't normally be hit but might occasionally be used, then the asymmetry of the second form matches the asymmetry of the logic.

doLotOfStuff();
if (condition) { 
    return foo; 
} 
return bar; 

But if either possibility is reasonable and it's simply a choice between them, I would allow the structure of the code to show that symmetry.

doLotOfStuff();
if (condition) { 
    return foo; 
} else {
    return bar; 
}

The code is there for the programmer to read, not the compiler.

Answer 2

It was once considered (and probably still be by some) that functions should have one entry point (easy but was relevant when you consider assembly language) and one exit point.

One exit point is nice from a debugging standpoint (as in you can put one watch/break on one line and know you'll go through it) but can lead to some horrific nesting so more often than not readability tends to win out. Which produces the least nesting, the least lines of code and the most readable end result? Ultimately that tends to be far more important than anything else.

For what it's worth the last can be better expressed as:

return condition ? foo : bar;

assuming condition isn't terribly long.

Don't get overly concerned with supposed code "purity". It's an irrelevant distraction. Make things readable and generally be consistent.

Answer 3

Well, I think that using multiple return is not readable.

I prefer code as:

String result = null;

doLotOfStuff();
if (condition) { 
    result = foo; 
} else {
    result = bar; 
}

return result;

Multiple returns is complicated to understand. But in your case I will prefer the postCheck

doLotOfStuff();
if (condition) { 
    return foo; 
} else {
    return bar; 
}

Answer 4

Well, some well-known Java experts argue that one should always strife to minimize the scope of things and that's also my opinion.

My teachers at university always tormented me that I must write stuff like this as posted by somebody before:

String result = null;
doLotOfStuff();
if (condition) {
    result = foo;
} else {
    result = bar;
}
return result;

The variable result has now a pretty big scope. In addition with a lot of nesting this tends to get very unreadable if you do a little bit more here, say you have another for-loop.

I think this is much better:

doLotOfStuff();
return condition ? foo : bar;

This is straight to the point. I see immediately what's going on, no checking of curly braces.

I think it's very good to

• Minimize scope
• Minimize indention (that is avoid curly braces)
• Minimize method length

The "Unnecessary else statement" warning helps with that.

Answer 5

I find this form

doLotOfStuff();
if (condition) { 
    return foo; 
}
return bar; 

to be more readable than the one with the else, it's less code and more intuitive if you think about it as a guard statement as in Fowler's Refactoring.

Question about multiple return points.

Answer 6

I know that without the IDE checking this for me I nearly always use the unnecessary else statement. Often I find it reads better initially with it, but usually remove it when it is pointed out as I can see the fact that it is unnecessary and it then bugs me...

Answer 7

Call me a bad boy but I would usually go with:

doLotOfStuff();
return condition ? foo : bar;

Single return statement, very readable.

Answer 8

In my opinion, the eclipse check is not very useful. What you should do depends on your program context. Else clauses are for alternatives (either this or that) whereas falling through to a return (no else clause) is more appropriate where you have a number of conditions that may be true and the fallthrough is more of a default reaction that you don't really invoke too often. So you use both in code that conveys meaning and as such checking for presence or absence is not useful.

Answer 9

Some of programmers claim that return in the middle of function is bad practice. Maybe for easy checking at function entry as in your 1st example it is OK and acceptable, but in other cases I would prefer setting result variable as in Jerome response.

Answer 10

If I trace the following code, I will notice that the first if-block is an exceptional code block. However, if I add an "else" to that, I may treat both are operational code block.

Also, for the main operational block, there may be some nested blocks. I prefer minimizing the nested level.

public String myMethod(input) {
    if (!validate(input)) {
        return "invalid input";
    }

    operation1();
    if (someCondition) {
        operation2();
    } else {
        operation3();
    }
    operation4();
    operation5();
    return operation6();
}