Flatten conditional as a refactoring

Question

Consider:

if (something) {
    // Code...
}

With CodeRush installed it recommended doing:

if (!something) {
    return;
}
// Code...

Could someone explain how this is better? Surely there is no benefit what so ever.

Answer 1

Isolated, as you've presented it - no benefit. But mark4o is right on: it's less nesting, which becomes very clear if you look at even, say a 4-level nesting:

public void foo() {
    if (a)
        if (b)
            if (c)
                if (d)
                    doSomething();
}

versus

public void foo() {
    if (!a)
        return;
    if (!b)
        return;
    if (!c)
        return;
    if (!d)
        return;
    doSomething();
}

early returns like this improve readability.

Answer 2

In some cases, it's cleaner to validate all of your inputs at the beginning of a method and just bail out if anything is not correct. You can have a series of single-level if checks that check successively more and more specific things until you're confident that your inputs are good. The rest of the method will then be much easier to write, and will tend to have fewer nested conditionals.

Answer 3

One less level of nesting.

Answer 4

This is a conventional refactoring meant for maintainability. See:

http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

With one condition, it's not a big improvement. But it follows the "fail fast" principle, and you really start to notice the benefit when you have lots of conditions. If you grew up on "structured programming", which typically recommends functions have single exit points, it may seem unnatural, but if you've ever tried to debug code that has three levels or more of nested conditionals, you'll start to appreciate it.

Answer 5

It can be used to make the code more readable (by way of less nesting). See here for a good example, and here for a good discussion of the merits.

That sort of pattern is commonly used to replace:

void SomeMethod()
{
    if (condition_1)
    {
        if (condition_2)
        {
            if (condition_3)
            {
                // code
            }
        }
    }
}

With:

void SomeMethod()
{
    if (!condition_1) { return; }
    if (!condition_2) { return; }
    if (!condition_3) { return; }

    // code
}

Which is much easier on the eyes.

Answer 6

I remember very clearly losing marks on a piece of college work because I had gone with the

if (!something) {
    return;
}
// Code...

format. My lecturer pontificated that it was bad practice to have more than one exit point in a function. I thought that was nuts and 20+ years of computer programming later, I still do.

To be fair, he lived in an era where the lingua franca was C and functions were often pages long and full of nested conditionals making it difficult to track what was going on.

Then and now, however, simplicity is king: Keeping functions small and commenting them well is the best way to make things readable and maintainable.

Answer 7

I don't think CodeRush is recommending it --- rather just offering it as an option.

Answer 8

IMO, it depends on if something or !something is the exceptional case. If there is a significant amount of code if something happens, then using the !something conditional makes more sense for legibility and potential nesting reduction.

Answer 9

Well, look at it this way (I'll use php as an example):

You fill a form and go to this page: validate.php

example 1:

<?php

if (valid_data($_POST['username'])) {

    if (valid_data($_POST['password'])) {

        login();

    } else {
        die();
    }

} else {
    die();
}

?>

vs

<?php

if (!valid_data($_POST['username'])) {
    die();
}

if (!valid_data($_POST['password'])) {
    die();  
}

login();

?>

Which one is better and easier to maintain? Remember this is just validating two things. Imagine this for a register page or something else.