Why is “else” rarely used after “if x then return”

Question

This method:

boolean containsSmiley(String s) {
    if (s == null) {
        return false;
    }
    else {
        return s.contains(":)");
    }
}

can equivalently be written:

boolean containsSmiley(String s) {
    if (s == null) {
        return false;
    }

    return s.contains(":)");
}

In my experience, the second form is seen more often, especially in more complex methods (where there may be several such exit points), and the same is true for "throw" as well as "return". Yet the first form arguably makes the conditional structure of the code more explicit. Are there any reasons to prefer one over the other?

(Related: Should a function have only one return statement?)

Answer 1

The second form if simpler/shorter. This doesn't always mean clearer. I suggest you do what you find clearest.. Personally I would write.

static boolean containsSmiley(String s) { 
    return s != null && s.contains(":)"); 
} 

Answer 2

I would prefer one exit point over multiple from maintenance perspective. Final result can be modified(or decorated) at one exit point rather than n exit points.

Answer 3

You will see this all over:

if (condition) {
    return var;
}
// by nature, when execution reaches this point, condition can only be false,
// therefore, the else is unnecessary
return other_var;

Most of the time the addition of an else clause is not only unnecessary in this case, but a lot of times, it gets optimized away by the compiler.

Think of how the computer thinks of this code (in terms of machine code, simplified into pseudocode here for demonstration purposes):

0x00: test [condition]
0x01: if result of test was not true, goto [0x04]
0x02: push [var] onto stack
0x03: goto [0x05]
0x04: push [other_var] onto stack
0x05: return from subroutine

The code (again, this is a pseudocode and not assembly) would act the exact same way for an if/then/else conditional.

It is, by many people, considered bad and/or confusing practice to have multiple possible exit points for a function, as a programmer must think of every possible path through his code. Another practice is the following:

return (condition) ? var : other_var;

This simplifies the code, and does not create any new exit points.

Answer 4

Cause it's nicer. You know you could also use '{' '}' to create several levels of nesting, but nobody really does it for just the heck of it.

Answer 5

This is a pattern called Guard Clause. The idea is do all the checking upfront to reduce nested conditions to increase readability.

From the link:

double getPayAmount() {
    double result;
    if (_isDead) {
        result = deadAmount();
    } else {
        if (_isSeparated) {
            result = separatedAmount();
        } else {
            if (_isRetired) {
                result = retiredAmount();
            } else {
                result = normalPayAmount();
            }
        }
    }

    return result;
}

Using the Guard Clause, you'll get to see this result:

double getPayAmount() {
    if (_isDead) return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired) return retiredAmount();

    return normalPayAmount();
};

Answer 6

Someone else probably noted this already, but I'd recommend against using null values in general where strings are expected. If you really want a check to prevent someone passing null values, you can use asserts (at dev time) or unit tests (deploy):

boolean containsSmiley(String s) {
    assert s != null : "Quit passing null values, you moron.";
    return s.contains(":)");
}

I've switched to a general rule of thumb: Never. Ever. pass null values, unless an external API calls explicitly asks for it. Second: If an external method may return null values, replace it with a sensible non-null value (such as an empty string) or add a neat check. I grow sick of repetitive if (thing == null) checks.

But that's a bit offtopic. I like putting short conditions on top and guard clauses, removing elses if program flow dictates it'll never get there.