Friday, July 2, 2010

Another style pet peeve

Here is another pet peeve people might do when program:

if(A) {
    if(B) {
        somecode1();
    }
}

Who knows, it might not matter the compiler, but for human readability sakes never do this, there really shouldn't be an excuse. Maybe for when debugging but in that case clean it up before you show it to the world.

Just write it as this:

if(A && B) {
    somecode1();
}

please...

Ok, so why write it as the latter? Well Personally every time I see nested if statement I think there is a reason for the nesting. There could be code at the start of nesting or at the end of the nesting, I can't tell at first glance, I must pay attention to those details, which is not something I'm good at, it also takes more time to investigate which of the damn closed nesting statements corresponds to which. Plus with more nesting = more text on the screen = harder to read.

Just remember that reading other peoples code is one of the hardest thing for a programmer to do, so the more to the point the code is written, the easier it is to read and understand (the whole point of reading).

how not to branch

As I work on code at work I see a constant theme, one that drives me crazy if goes like this:

for(A){
    if(B) {
        somecode1();
        if(!C)  {
            somecode2();
            if(D || E) {
                somecode3();
            }
        }
    }
}


As you can see it's starting to look like a Christmas tree which isn't very readable. Instead one should write (or refactor) the above code to look more like this:

for(A){
    if(!B) continue;
    somecode1();
    
    if(C) continue;
    somecode2();

    if(!D && !E) continue;
    somecode3();
}

My instructor in college (Dr. Hunter) referred to these as sentinel statements, or something like them. Nesting is not the easiest way to read code and betrays organization. One intends that somecode1-3 gets called in order unless some of the conditions occur. Putting them all in the same indentation gives the reader of your code that impression, by placing code in a nest it's as if your saying "hey this code is one part that could be run, but isn't necessarily what the point of this method is" So I say, indent the optional code, don't for intended code.