6

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.

  • 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" – LennyProgrammers Jun 29 '11 at 8:02
  • If this is your biggest worry in your current job then you are probably ready for that promotion! – Mongus Pong Jun 29 '11 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. – The Muffin Man Jun 29 '11 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. – Rice Flour Cookies Dec 7 '11 at 16:18
8

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.

  • 10
    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 Arnold Aug 1 '11 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 Aug 9 '11 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 Oct 27 '15 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.

  • 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 Jun 29 '11 at 17:15
  • 1
    Also, the nice thing is that this approach highlights the "default-ness" of the return value. – Stephen Gross Dec 7 '11 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 Jan 14 '15 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. – Adrian McCarthy Dec 19 '17 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.

  • 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; } – Carlos Campderrós Jun 29 '11 at 8:53
  • 2
    Carlos: That would also be a possibility, but in general, I rather avoid adding more variables than necessary. – user281377 Jun 29 '11 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
    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. – Sheldon Warkentin Dec 7 '11 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 Dec 8 '11 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.

  • 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 Dec 7 '11 at 15:42
  • 2
    @Ryathal, I don't see any ambiguity there. – Abyx Dec 7 '11 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. – Adrian McCarthy Dec 19 '17 at 0:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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