Best practice in setting return value (use else or?)

Question

Whenever you want to return a value from a method, but whatever you return depends on some other value, you typically use branching:

int calculateSomething() {
 if (a == b) {
 return x;
 } else {
 return y;
 }
}

Another way to write this is:

int calculateSomething() {
 if (a == b) {
 return x;
 }
 return y;
}

Is there any reason to avoid one or the other? Both allow adding "else if"-clauses without problems. Both typically generate compiler errors if you add anything at the bottom.

Note: I couldn't find any duplicates, although multiple questions exist about whether the accompanying curly braces should be on their own line. So let's not get into that.

• Kramii: Reated: programmers.stackexchange.com/questions/77530/… and stackoverflow.com/questions/36707/…
• LennyProgrammers: I think it's just a matter of taste. After some serious exposure to Haskell i find the former easier to read and understand. I use the latter only for precondition-checks like "if a is None: return None"
• Mongus Pong: If this is your biggest worry in your current job then you are probably ready for that promotion!
• The Muffin Man: I use both depending on what I feel is more readable. Although if I have one big block of code that's within in if statement I use a return before the block rather than wrapping the entire statement.
• Vivian River: I use both interchangeably. When I use Visual Studio, I usually also set the "Treat Warnings as Errors" options which will help me catch a lot return statement related control mistakes at compile-time.

Answer 1

I use both, depending on the method. For example, in a method that decides something, i think that

public boolean isSunday(Calendar cal) {
 if (Calendar.SUNDAY == cal.get(Calendar.DAY_OF_WEEK))
 return true;
 else
 return false;
}

captures the intent of the method better. It is stateless, it is not supposed to be extended, and it is closed in that the if-clause generates all values the method may ever return.

If the method calculates something, or retrieves information, i like the second variant more:

public String getAnswer(int question) {
 if (isAvailable(preciseAnswerService)
 return preciseAnswerService.answer(question);
 return defaultAnswer(question);
}

Here, the fact that one answer is chosen over the other depends on the state of the answerService. We might extend the method later by adding a preciseAnswerCache, or retrieve the answer from somewhere else.

• Andrew: For the first one, why not just return Calendar.SUNDAY == cal.get(Calendar.DAY_OF_WEEK)? Something that simple doesn't even need an if-else.
• wallenborn: I agree, if it's that simple, a one-liner is best, maybe i should've picked a better example.
• pnizzle: I honestly think that this is a good example of where neither is better than the other, and instead is a matter of sticking to one practice instead of mixing. Brace position, now that can start a war.

Answer 2

I prefer a third approach, especially when dealing with Booleans:

int calculateSomething() { 
 int value = y; // whatever the default value is
 if (a == b) { 
 value = x;
 }
 return value;
}

I find it more natural in that you don't have return statements everywhere in the logic flow, however there are times when it's better to use the second form. The above is just my preference by default.

• ToddR: I tend to prefer this approach. I find that fewer returns makes it easier for me to follow the code. An exception is when I have to test an argument and wrap a lot of code in the if block if the argument is good, or simply return error if it's bad. In that case I'll probably test for the bad case up front and return right away. The good path follows with one less level of nesting to worry about.
• Stephen Gross: Also, the nice thing is that this approach highlights the "default-ness" of the return value.
• Florian F: It is also easier to debug when you have a single exit point. You can set a breakpoint on the return statement and see if the returned value is what you expect.
• Adrian McCarthy: One return at the end also makes it easy to assert post-conditions. This is one of the reasons I prefer this in general, but it varies by language, the style of the local style of the people I'm working with, the complexity of the function, and sometimes the semantics.

Answer 3

I prefer to have one return clause that is not inside a loop, if- or else-branch. Therefore, I opt for the second version.

My reason: In PL/SQL (and probably other languages too), the compiler doesn't notice the omission of a return statement, but the program throws an exception if a function doesn't end with an explicit return statement.

• Carlos Campderrós: But then, shouldn't you dislike second version too and prefer using a variable? function doSomething() { bool x; if (a == b) { x = true; } else { x = false; } return x; }
• user281377: Carlos: That would also be a possibility, but in general, I rather avoid adding more variables than necessary.

Answer 4

Like Lenny222 says in the comments, it is indeed just a matter of taste.

I personally prefer the second form, as it is

• more concise
• better readable (imo)

I think the else-clause in the first form is just plain redundant, and so adds clutter.

Answer 5

In Java you can write :

int calculateSomething() { 
 return a == b ? x : y;
}

• Sheldon Warkentin: Shorthand in Java and C# is good for small expressions (like the one you have listed). If each result and the conditional become any more complex, it becomes difficult to decipher.
• cl-r: You are right : if you have 'nested' if like "a== b?(a>c?:(?:)):(?:)", coding with {} may be 2 time faster and naturally more easy to decipher

Answer 6

There is no point to use else after return.

LLVM Coding Standards says the same:

...do not use 'else' or 'else if' after something that interrupts control flow — like return, break, continue, goto, etc. For example, this is bad:

case 'J': {
 if (Signed) {
 Type = Context.getsigjmp_bufType();
 if (Type.isNull()) {
 Error = ASTContext::GE_Missing_sigjmp_buf;
 return QualType();
 } else {
 break;
 }
 } else {
 Type = Context.getjmp_bufType();
 if (Type.isNull()) {
 Error = ASTContext::GE_Missing_jmp_buf;
 return QualType();
 } else {
 break;
 }
 }
}

...The idea is to reduce indentation and the amount of code you have to keep track of when reading the code.

• Ryathal: its far better to have the else there than have a potentially ambiguous statement that can be made much more clear with so little effort
• Abyx: @Ryathal, I don't see any ambiguity there.
• Adrian McCarthy: It's a widely-believed myth that early-returns and continues are the only (or best) way to avoid excessive indentation. This is probably my least-favorite aspect of LLVM style.