7

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.

7
  • 1
  • 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" Commented Jun 29, 2011 at 8:02
  • If this is your biggest worry in your current job then you are probably ready for that promotion! Commented Jun 29, 2011 at 8:34
  • 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. Commented Jun 29, 2011 at 9:50
  • 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. Commented Dec 7, 2011 at 16:18

6 Answers 6

9

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.

3
  • 11
    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.
    – Andrew
    Commented Aug 1, 2011 at 15:40
  • 1
    I agree, if it's that simple, a one-liner is best, maybe i should've picked a better example.
    – wallenborn
    Commented Aug 9, 2011 at 15:50
  • 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.
    – pnizzle
    Commented Oct 27, 2015 at 1:31
10

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.

4
  • 1
    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.
    – ToddR
    Commented Jun 29, 2011 at 17:15
  • 1
    Also, the nice thing is that this approach highlights the "default-ness" of the return value. Commented Dec 7, 2011 at 15:50
  • 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.
    – Florian F
    Commented Jan 14, 2015 at 17:52
  • 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. Commented Dec 19, 2017 at 0:41
5

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.

2
  • 1
    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; } Commented Jun 29, 2011 at 8:53
  • 2
    Carlos: That would also be a possibility, but in general, I rather avoid adding more variables than necessary.
    – user281377
    Commented Jun 29, 2011 at 8:56
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.

4

In Java you can write :

int calculateSomething() { 
    return a == b ? x : y;
}
2
  • 2
    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. Commented Dec 7, 2011 at 15:25
  • 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
    – cl-r
    Commented Dec 8, 2011 at 16:01
1

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.

3
  • 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
    – Ryathal
    Commented Dec 7, 2011 at 15:42
  • 2
    @Ryathal, I don't see any ambiguity there.
    – Abyx
    Commented Dec 7, 2011 at 16:04
  • 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. Commented Dec 19, 2017 at 0:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.