Why is "else" rarely used after "if x then return"?

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 co开发者_如何学JAVAmplex 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?)

The else in that case would be redundant, as well as create unnecessary extra indentation for the main code of the function.

In my experience, it depends on the code. If I'm 'guarding' against something, I'll do:

if (inputVar.isBad()) {
    return;
}

doThings();

The point is clear: If that statement is false, I don't want the function to continue.

On the other hand, there are some functions with multiple options, and in that case I would write it like this:

if (inputVar == thingOne) {
    doFirstThing();
} else if (inputVar == secondThing) {
    doSecondThing();
} else {
    doThirdThing();
}

Even though it could be written as:

if (inputVar == thingOne) {
    doFirstThing();
    return;
}
if (inputVar == thingTwo) {
    doSecondThing();
    return;
}
doThingThree();
return;

It really comes down to which way most clearly shows what the code is doing (not necessarily which bit of code is shortest or has the least indentation).

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();
};

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.

I prefer writing it like this:

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

The else is redundant. Also some IDEs (Eclipse) and analysis tools (maybe FindBugs) may flag that as a warning or an error, so in that case programmers are likely to remove it.

Like any "discussion" about coding styles there is no correct answer. I prefer to apply these considerations:

1. Does the code work as expected in all situations. (Principle of least surprise)

2. Can the next developer (myself or someone else) understand what it is doing and why.

3. How fragile is the code with respect to change.

4. Is is simple as it needs to be and no more. I.e. no over or under engineering.

Once I'm happy that I have satisfied the above, the rest generally just falls falls into line.

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.

I would prefer the first option, as it is more human-readable.

As an analogy, compare the next 2 sentences: "If today is raining, then take an umbrella, else take sunglasses." and "If today is raining, then take an umbrella, take sunglasses". The first sentence corresponds to the first block of code from the question, the second one – to the second. The first one is much more clear and readable, isn't it?

It's religious argument and at the end of the day it doesn't matter. I'd even argue that the first form is more readable in some circumstances. If you have large chunks of code in an if-elseif-elseif-else, it's easier, at first glance to see what the default return is.

if (s == null) {
    return false;
}
else if (s.Contains(":))")) {
    return true;
}
else if (s.Contains(":-(")) {
    return false;
}

return s.contains(":)");

Occam's Razor is the principle that "entities must not be multiplied beyond necessity."

The if statement is checking/enforcing your contract/expectation of not receiving null values. For that reason, I would prefer to see it separated from the rest of the function as it doesn't have anything to do with the actual logic of what you're trying to accomplish (though this case is very simple).

In most cases, though, I prefer code to be as explicit as possible in its intent. If there's something that you can restructure about your function to make it more readable for others, do it. As a professional programmer, your goal should really be to program for those who have to maintain your code after you (including yourself 2 years later...). Anything you can do to help them out is worth doing.

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.

While having an else is correct and there's nothing wrong with it in terms of logic and runnability, I like to avoid the initial WTF moment when the function has no return statement outside of the if/else scope.

The first form is simply less verbose - when you return a value you automatically leave the scope of the function you're in and return to the caller, so any of the code thereafter will only execute if the IF statement doesn't evaluate to true and subsequently return anything.

I'd argue for readability. If you're scanning screens of code trying to figure out what the code does, it's a visual prompt to the developer.

...but it's not really needed because we all comment our code so well, right? :)

In my opinion, the second one makes more sense. It serves as more of a 'default' action, like a switch. If it doesn't match any of the other exit points, then do that. You don't really need an else there. I would say if the entire function is only if and elseif, then an else would make sense there because it's one giant conditional. If there's multiple conditionals and other functions that are run within it, then a default return at the end would be used.

As you can see, different people have different opinions on readability. Some people think that fewer lines of code tends to make the code more readable. Others think that the symmetry of the second form makes it more readable.

My take is that probably, both views are correct ... for the people who hold them. And the corollary is that you cannot write code that everyone finds optimally readable. So, the best advice is to follow what your mandated coding standard says to do (if it says anything on this) and generally use your common sense. (If you are burdened with some vociferous nitwit that insists that his way is "right" ... just go with the flow.)

Because there is an optional (switched off by default) warning in eclipse if else is used in such situation ;).

Well, some of the reason is just convention, but there is one advantage to the form above...

It's typical when coding a return statement, to make the last statement your default return value. This primarily aids during refactoring - else clauses tend to get wrapped up in other structures, or might accidentally be moved deeper into the tree.

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.

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(":)"); 
} 

Because it's the same as writing one of the following which brings the evidence about the intention of the programmer:

boolean containsSmiley(String s) {
    if (s == null)   // The curly braces are not even necessary as the if contains only one instruction.
        return false;

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

Or even this:

boolean constainsSMiley(String s) {
    return string.IsNullOrEmpty(s) ? false : s.Contains(":)");
}

These two forms are:

1. More elegant;
2. Easier to read;
3. Leaner and swifter for the reading programmer.