Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Early returns vs nested positive if statements

Tags:

coding-style

Here is some hypothetical code sample:

if (e.KeyCode == Keys.Enter)
{
    if (this.CurrentElement == null) {
        return false;}

    if (this.CurrentElement == this.MasterElement) {
        return false;}

    if (!Validator.Exist (this.CurrentElement)) {
        return false;}

    if (!Identifier.IsPictureElement (this.CurrentElement)) {
        return false;}

    this.FlattenObjects(this.CurrentElement);
}

VS

if (e.KeyCode == Keys.Enter)
{
    if (this.CurrentElement != null) {

        if (this.CurrentElement != this.MasterElement) {

            if (Validator.Exist (this.CurrentElement)) {

                if (Identifier.IsPictureElement (this.CurrentElement)) {

                    this.FlattenObjects(this.CurrentElement);}}}}}}

}

Which one do you think is better in terms of readability, maintenance, etc?

Also the 2nd example can be formatted differently via the different use of parenthesis.

like image 744
Joan Venge Avatar asked Dec 06 '10 18:12

Joan Venge


People also ask

Why would you use a nested IF statement instead of an if else?

A nested if statement is an if statement placed inside another if statement. Nested if statements are often used when you must test a combination of conditions before deciding on the proper action.

Should I use early return?

An early return, or “return early” is an approach to keep readability in functions and methods. It is always considered a wise choice to return early if simple conditions apply that can be checked at the beginning of a method.

Should you avoid nested if statements?

Avoid using nested if-else statements. Keep the code linear and straightforward. Utilize creating functions/methods. Compare it when we try to use an if-else statement that is nested and that does not utilize the power of the return statement, We get this (Code 1.4).

What is an early return?

Early return is a situation where the renter returns the rental vehicle before the agreed drop-off time. The supplier may give the renter a refund or charge an Early Return Fee, depending on the supplier's terms and conditions.


2 Answers

Early returns are much more readable.

Whenever you get more than four or five levels of nesting inside a method, it's time to refactor that method.

A single if with an || clause can sometimes be more readable:

if (this.CurrentElement == null
 || this.CurrentElement == this.MasterElement
 || !Validator.Exist(this.CurrentElement)
 || !Identifier.IsPictureElement(this.CurrentElement))
    return false;
like image 97
SLaks Avatar answered Dec 15 '22 14:12

SLaks


The first example is better in every way. It's simpler, and easier to read. Some people say that every function should have a single return point; this example shows clearly why those people are wrong.

PS Personally I would remove all those superfluous curly brackets:

if (this.CurrentElement == null) return false;

etc. This makes it even simpler, and even easier to read.

like image 43
TonyK Avatar answered Dec 15 '22 12:12

TonyK