When returning inside of an if block, should an else block be used? [closed]

Question

Let's say I'm writing a method which can return two very different things based on parameters. I could write that bit of code like this:

function getMyThing(a) {
    if (a == 0) {
        // Do some stuff
        return b;
    } else {
        // Do some stuff
        return c;
    }
}

This piece of code is clear and concise. It's easy to see the program flow. But it could also be written like this:

function getMyThing(a) {
    if (a == 0) {
        // Do some stuff
        return b;
    }
    // Do some stuff
    return c;
}

To me, this bit of code seems less clear, but it also seems slightly more efficient. While I understand that such a minor optimization is usually irrelevant, it would bug me if I didn't know the answer. Is this sort of thing optimized by the compiler? If not, which is the preferred method of writing code?

Answer 1

I suggest the following approach. I think it's always a good idea to have a consistent structure (for methods and class). Having a consistent structure ensure that code is readable, and you know where to look for when you read the code with a specific goal.

For method i use the following structure:

1. Local variable declaration at top

2. Precondition check

3. Initialization

4. Method processing

5. Return the value

Example:

function getMyThing(a) {
  int result = 0;

  // Precondition check
  if (a == -1)  return None;

  if (a == 0) {         
   // Do some stuff         
   result = b;     
  }  else {         
    // Do some stuff         
   result = c;     
 } 

 return result;
 } 

Answer 2

I used to always omit the else thinking I was being super-cool, but now I switch depending on the context.

When the if test is picking between two (or more for if-elseif-else) options, I use if-else for clarity. In fact, if there are two short choices, I prefer the ternary operator here when available in the language.

function getMyThing(a) {
    return a == 0 ? b : c;
}

When the if is checking for an error condition or invalid value and either returning or throwing an exception, I omit the else.

function getMyThing(a) {
    if (a == 0) {
        throw new InvalidArgumentException("a must be non-zero");
    }
    ...
}

Answer 3

In C#, the ReSharper plug-in will tell you that the else is redundant if the method exits (returns or throws) from within the if block. It's not an efficiency thing, per se, because this is a very obvious thing for good compilers to optimize.

In .NET at least, an if-else block is constructed by evaluating the condition (or two values that will be compared), pushing the result onto the evaluation stack, then performing a command (one of the "break" opcodes) that will jump directly to the else block if the opposite of the condition is true. Otherwise, the condition itself is true and the program falls through into the code for the if block, and then at the end of that block is a "break" statement that jumps unconditionally beyond the end of the else block to the next statement after the whole thing.

Now, if you return or throw out of the if block, then any break statement to jump over the else block will never be executed. The .NET compiler is smart enough to realize this, and doesn't put the break statement at the end of the if block if there's a ret or throw command preceding it. So, whether you use the else block or not, the IL is identical in this situation, hence the "warning" from ReSharper that it's redundant. But, again because they're identical, if you want to use the else block it's your prerogative.

Answer 4

question of faith or code readability

Structured programming somehow brought up the principle of one return statement/a single exit point. But: technically it's not needed with modern programming (languages).

Personally I prefer using early returns because you can flatten your code.

A single return provokes the arrow-code anti pattern: http://c2.com/cgi/wiki?ArrowAntiPattern

• http://www.leepoint.net/JavaBasics/methods/method-commentary/methcom-30-multiple-return.html
• http://www.codinghorror.com/blog/2006/01/flattening-arrow-code.html

Answer 5

For my opinion, if a precondition check is violated, then return early (as devsundar's reply). However, it sounds as though this is not what you meant (which can return two very different things based on parameters).

In this case, I'd say that they shouldn't be handled within the same function, and I'd treat it as something like:

function getMyThing(a) {
    switch ( some_expression_based_on_parameters )
    {
        case action_1: return HandleAction1(...);
        case action_2: return HandleAction2(...);
        // etc
        default: throw UnknownActionException(...);
    }
}

Answer 6

If you have something equivalent like we have in C# I would suggests you sue use that if clearly tells you the return on boolean condition

function getMyThing(a) {

    return (a == 0) ? StuffWithtB() : StuffWithC();
}