Invert “if” statement to reduce nesting

Question

When I ran ReSharper on my code, for example:

    if (some condition)
    {
        Some code...            
    }

ReSharper gave me the above warning (Invert "if" statement to reduce nesting), and suggested the following correction:

   if (!some condition) return;
   Some code...

I would like to understand why that's better. I always thought that using "return" in the middle of a method problematic, somewhat like "goto".

Answer 1

Personally I prefer only 1 exit point. It's easy to accomplish if you keep your methods short and to the point, and it provides a predictable pattern for the next person who works on your code.

eg.

 bool PerformDefaultOperation()
 {
      bool succeeded = false;

      DataStructure defaultParameters;
      if ((defaultParameters = this.GetApplicationDefaults()) != null)
      {
           succeeded = this.DoSomething(defaultParameters);
      }

      return succeeded;
 }

This is also very useful if you just want to check the values of certain local variables within a function before it exits. All you need to do is place a breakpoint on the final return and you are guaranteed to hit it (unless an exception is thrown).

Answer 2

My idea is that the return "in the middle of a function" shouldn't be so "subjective". The reason is quite simple, take this code:

    function do_something( data ){

      if (!is_valid_data( data )) 
            return false;


       do_something_that_take_an_hour( data );

       istance = new object_with_very_painful_constructor( data );

          if ( istance is not valid ) {
               error_message( );
                return ;

          }
       connect_to_database ( );
       get_some_other_data( );
       return;
    }

Maybe the first "return" it's not SO intuitive, but that's really saving. There are too many "ideas" about clean codes, that simply need more practise to lose their "subjective" bad ideas.

Answer 3

A return in the middle of the method is not necessarily bad. It might be better to return immediately if it makes the intent of the code clearer. For example:

double getPayAmount() {
    double result;
    if (_isDead) result = deadAmount();
    else {
        if (_isSeparated) result = separatedAmount();
        else {
            if (_isRetired) result = retiredAmount();
            else result = normalPayAmount();
        };
    }
     return result;
};

In this case, if _isDead is true, we can immediately get out of the method. It might be better to structure it this way instead:

double getPayAmount() {
    if (_isDead)      return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired)   return retiredAmount();

    return normalPayAmount();
};   

I've picked this code from the refactoring catalog. This specific refactoring is called: Replace Nested Conditional with Guard Clauses.

Answer 4

Performance-wise, there will be no noticeable difference between the two approaches.

But coding is about more than performance. Clarity and maintainability are also very important. And, in cases like this where it doesn't affect performance, it is the only thing that matters.

There are competing schools of thought as to which approach is preferable.

One view is the one others have mentioned: the second approach reduces the nesting level, which improves code clarity. This is natural in an imperative style: when you have nothing left to do, you might as well return early.

Another view, from the perspective of a more functional style, is that a method should have only one exit point. Everything in a functional language is an expression. So if statements must always have an else clauses. Otherwise the if expression wouldn't always have a value. So in the functional style, the first approach is more natural.

Answer 5

Multiple return points were a problem in C (and to a lesser extent C++) because they forced you to duplicate clean-up code before each of the return points. With garbage collection, the try | finally construct and using blocks, there's really no reason why you should be afraid of them.

Ultimately it comes down to what you and your colleagues find easier to read.

Answer 6

I'd like to add that there is name for those inverted if's - Guard Clause. I use it whenever I can.

I hate reading code where there is if at the beginning, two screens of code and no else. Just invert if and return. That way nobody will waste time scrolling.

http://c2.com/cgi/wiki?GuardClause