Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
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
show 1 more comment

6 Answers

up vote 7 down vote accepted

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.

share|improve this answer
8  
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
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
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
add comment

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.

share|improve this answer
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
add comment

In Java you can write :

int calculateSomething() { 
    return a == b ? x : y;
}
share|improve this answer
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
add comment

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.

share|improve this answer
    
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
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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