If-Then-Else Return Style

Question

Possible Duplicate:
Should a function have only one return statement?

While reading the section on refactoring in Code Complete by Steve McConnell, I noticed a suggestion that ran contrary to what I personally believed. McConnell suggests that during a nested if-else-then block you should return immediately after calculating the answer. Prior to this, I designed my code to assign the result to a return variable and at the end of the routine return the variable. The following are two examples using both methods.

My Style:

public int fibonacciSeq( int n ) {  
  int result = 0;  
  if( n == 1 || n == 2 ) {  
    result = 1;  
  } else if( n > 2 ) {  
    result = fibonacciSeq( n - 1 ) + fibonacciSeq( n - 2 );  
  } else {  
    result = 0;  
  }  
  return result;  
}

McConnell’s Style:

public int fibonacciSeq( int n ) {  
  if( n == 1 || n == 2 ) {  
    return 1;  
  } else if( n > 2 ) {  
    return fibonacciSeq( n - 1 ) + fibonacciSeq( n - 2); 
  }  
  return 0;  
}

For the purpose of showing the two differences, I ignored the performance liability that this algorithm holds (both have running complexity of O(n^2)). Which of those is actually easier to read, and in a far more complex situation, would this still have the same answer?

--Edit-- Also requested is why is the answer the case?

Answer 1

I prefer having multiple return statements, to be honest.

In an if-then-else style they might be basically equivalent, but in a longer function you don't want to fall out of the if and run through the rest of the function if you really want a premature return.

Compare:

char *getRandomString() /* McConnell */
{
  if (someCondition) return NULL;

  /* do long computation */
  return someString;
}

versus

char *getRandomString() /* OP's style? */
{
  char *result;
  do
  {
    if (someCondition)
    {
      result = NULL;
      break;
    }

    /* do long computation */

  } while (0);

  return result;
}

That break statement is in NO way intuitive. And it's either use a run-once loop or use GOTOs. Nobody likes GOTOs.

It's much better to return when you know your result, I find.

Answer 2

I believe that it depends totaly from the situation:
For example in this snippet:

SomeClass& operator=(const SomeClass& pattern)
{
if (this != &pattern)
{
//do something
}
return *this;
}
//Instead of this which is obviously worse
SomeClass& operator=(const SomeClass& pattern)
{
if (this == &pattern)
{
return *this;
}
else
{
//do something and
return *this;
}
}

But there are situation where multiple return's are preferable.

Answer 3

"One entry, one exit" is one of those rules from back in the day when code could jump into the middle of a function or jump out at will. (Old-school BASIC and assembly language spring to mind.)

While it's still good to keep the number of entry and exit points down, there are lots of cases where an immediate return is more readable than setting a flag and checking it 20 lines later. In those cases, return. Don't contort the logic just to make it fit some guideline made 30+ years ago.

Answer 4

The answer, as always, is "it depends."

That said, I do have a specific example where the multiple returns make your life a little bit harder: logging.

There are times in the development of a large complex system where you need to monitor the progress, state or just generally debug what's going on. Obviously, this is a simplistic example. However, there are plenty of times when you need to say something to the user along the lines of:

// ...
} else if( n > 2 ) {  
    result = fibonacciSeq( n - 1 ) + fibonacciSeq( n - 2 );  
    if (logger.isDebugEnabled()) {
        logger.debug("Using option two: result = " + result);
    }
} else {
// ...

That sort of logging is still possible, of course, but less convenient.

In the end, I tend to prefer the easiest style to debug. That's going to be very subjective and will depend on the team, the code, the problem being solved and, quite likely, my mood that day.