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

Performance is in two parts. You have performance when the software is in production, but you also want to have performance while developing and debugging. The last thing a developer wants is to "wait" for something trivial. In the end, compiling this with optimization enabled will result in similar code. So it's good to know these little tricks that pay off in both scenarios.

The case in the question is clear, ReSharper is correct. Rather than nesting if statements, and creating new scope in code, you're setting a clear rule at the start of your method. It increases readability, it will be easier to maintain, and it reduces the amount of rules one has to sift through to find where they want to go.

Answer 2

As others have mentioned, there shouldn't be a performance hit, but there are other considerations. Aside from those valid concerns, this also can open you up to gotchas in some circumstances. Suppose you were dealing with a double instead:

public void myfunction(double exampleParam){
    if(exampleParam > 0){
        //Body will *not* be executed if Double.IsNan(exampleParam)
    }
}

Contrast that with the seemingly equivalent inversion:

public void myfunction(double exampleParam){
    if(exampleParam <= 0)
        return;
    //Body *will* be executed if Double.IsNan(exampleParam)
}

So in certain circumstances what appears to be a a correctly inverted if might not be.

Answer 3

Guard clauses or pre-conditions (as you can probably see) check to see if a certain condition is met and then breaks the flow of the program. They're great for places where you're really only interested in one outcome of an if statement. So rather than say:

if (something) {
    // a lot of indented code
}

You reverse the condition and break if that reversed condition is fulfilled

if (!something) return false; // or another value to show your other code the function did not execute

// all the code from before, save a lot of tabs

return is nowhere near as dirty as goto. It allows you to pass a value to show the rest of your code that the function couldn't run.

You'll see the best examples of where this can be applied in nested conditions:

if (something) {
    do-something();
    if (something-else) {
        do-another-thing();
    } else {
        do-something-else();
    }
}

vs:

if (!something) return;
do-something();

if (!something-else) return do-something-else();
do-another-thing();

You'll find few people arguing the first is cleaner but of course, it's completely subjective. Some programmers like to know what conditions something is operating under by indentation, while I'd much rather keep method flow linear.

I won't suggest for one moment that precons will change your life or get you laid but you might find your code just that little bit easier to read.

Answer 4

In my opinion early return is fine if you are just returning void (or some useless return code you're never gonna check) and it might improve readability because you avoid nesting and at the same time you make explicit that your function is done.

If you are actually returning a returnValue - nesting is usually a better way to go cause you return your returnValue just in one place (at the end - duh), and it might make your code more maintainable in a whole lot of cases.

Answer 5

I think it depends on what you prefer, as mentioned, theres no general agreement afaik. To reduce annoyment, you may reduce this kind of warning to "Hint"

Answer 6

The idea of only returning at the end of a function came back from the days before languages had support for exceptions. It enabled programs to rely on being able to put clean-up code at the end of a method, and then being sure it would be called and some other programmer wouldn't hide a return in the method that caused the cleanup code to be skipped. Skipped cleanup code could result in a memory or resource leak.

However, in a language that supports exceptions, it provides no such guarantees. In a language that supports exceptions, the execution of any statement or expression can cause a control flow that causes the method to end. This means clean-up must be done through using the finally or using keywords.

Anyway, I'm saying I think a lot of people quote the 'only return at the end of a method' guideline without understanding why it was ever a good thing to do, and that reducing nesting to improve readability is probably a better aim.