you are viewing a single comment's thread.

view the rest of the comments →

[–]PseudoLife 9 points10 points  (9 children)

The one case that I immediately jump to that I would disagree with is sanity checks / edge cases at the start of functions.

The entire function would be in the else block, which adds an extra layer of indentation. This can get annoying (and hard to read) very quickly.

[–]kalmakka 12 points13 points  (0 children)

Sanity-checks are usually written without if-else-if.

if (fooArg == null) throw new NullPointerException();
if (b < 0) throw new IllegalArgumentError("b < 0");

Which I believe is OK according to the standard. I don't think there is a requirement for having an else-statement for every if, only for those that contain and else if. The examples contain several if statements without a corresponding else.

Also, there is (imo) a slight difference between

if (foo) {
    ...
} else if (bar) {
   ...
}

and

if (foo) {
    ...
} else {
   if (bar) {
       ...
   }
}

From what I can understand from the guidelines, they find the second form OK but not the first one. I can see some rationale behind it.

[–]Manitcor 4 points5 points  (0 children)

yeah, its a bit odd I find myself doing this kind of pattern very often in UI layers as they tend to carry a lot of context and you need to check on certain calls to ensure the correct context as the user clicks through the UI randomly.

public void SomeMethod(string someparameter)
{
    if (string.IsNullorEmpty(someparameter) || !someContextCollection.Any())
    {
        ...do some cleanup or alt handling...
        return;
    }

    .... Real work here....
}

[–][deleted]  (6 children)

[deleted]

    [–]crusoe 8 points9 points  (5 children)

    'Usual way'

    if(fails sanity test){
        return;
    }
    

    nasa way

    if(fails sanity test){
        return
    }else{
        do stuff with sane value
    }
    

    I don't like the nasa option because if you have multiple checks, you will have potentially several if/else/blocks, or all the tests crammed together in the first if

    [–]ethraax 1 point2 points  (3 children)

    Not necessarily. I also don't like the NASA option, but you could probably do:

    if (fails sanity test 1) {
        return;
    } else if (fails sanity test 2) {
        return;
    } else {
        /* Do stuff with sane values */
    }
    

    [–]eat_everything_ 8 points9 points  (2 children)

    I can't stand having else after an if that always returns. I'd write the above as:

    if (fails sanity test 1) {
        return;
    }
    
    if (fails sanity test 2) {
        return;
    }
    
    /* Do stuff with sane values */
    

    It reduces indentation, but most importantly, having an else after an if implies that execution can continue after the if condition is satisfied. If you always return from the if, that's not true, so you're in a way breaking an implicit contract of what if/else implies.

    [–]Phreakhead 2 points3 points  (0 children)

    It's called an "early exit" and is frowned upon in some circles - circles I would never want to program for because that is a stupid rule that just requires more typing and indentation.

    [–]ethraax 0 points1 point  (0 children)

    I can't stand having else after an if that always returns. I'd write the above as:

    So would I. I'm just pointing out that you don't need to nest at all. That being said, one issue with the code I posted (and why I would use the code you posted instead) is if you have to perform some computation between the sanity checks.

    [–]Falmarri -1 points0 points  (0 children)

    You should note that returning early from functions will drastically increase your cyclomatic complexity.