Background
This probably sounds really nit-picky or OCD, but I think it’s an issue worth addressing. Excessive nesting of logic within code can make things nightmarish to read. Even a few of years ago I never thought anything of this. I mean, how much could it really affect someone reading it? He/she must be a complete newb to not be able to read my logic. Fast forward to a co-op placement where this was more closely moderated by my managers, and I began to pay more attention to it…
Why?
Alright, so all that you know so far about my opinion on this is that excessive nesting bothers me. So far, my mission is accomplished. Everything else is just extra. The first issue with excessive nesting is that it actually makes logic hard to follow. If you’re doing code reviews or revisiting your old code, large methods that have lots of nested if statements and loops actually become a tangled mess of logical workflows. You don’t need to believe me yet, but I’m hoping by the end of this you might change your mind.
The next thing, and it’s related, is that it makes refactoring code quite tricky. If you have lot’s of deeply nested if statements, switching up the behaviour of a function even a little bit could have your mind warping with how to tackle all the logical branches. Have fun. Remember that one monolithic function that nobody wanted to go back and refactor? Well, it turns out you need to pass in another parameter now and handle it in all of your separate logical paths. Hold back the tears when you’re trying to recall the logic once you’re 10+ levels deep into nested if statements.
Another key point I’d like to mention is that, in my opinion, the larger the vertical separation between a conditional check and it’s bodies (i.e. the if block and the else block) the more difficult it becomes to read the code. Of course, this may not be a law or an all-the-time thing, but it’s certainly a decent guideline. Think about it though. If you have an enormous block of code for your if statement body, by the time you finish understanding that, you have to go back up to the if statement condition and invert the whole thing to beign to understand what your else block does.
The Offender
Let’s have a look at some real offensive code. Who knows what it does really… Well, nobody does. Why? Because it’s completely contrived to illustrate my point. And that’s that. Behold the horror!
private void DoStuff()
{
foreach (thing in thisList)
{
if (condition1)
{
if (condition2)
{
DoThis(thing);
}
else
{
if (condition3)
{
continue;
}
else
{
if (condition4)
{
continue;
}
else
{
if (condition5)
{
if (condition6)
{
continue;
}
else
{
if (condition7)
{
continue;
}
else
{
if (condition8)
{
DoThis(thing);
}
}
}
}
}
}
}
}
else
{
DoThis(thing);
}
}
}Pretty filthy, right? In all honesty, anyone who has worked in production code is guaranteed to have seen code that nests much much deeper… veering right off the developer’s window (and some of us code with multiple monitors). It’s a scary world out there, and this example doesn’t even begin to illustrate how bad it can get. I mean, this particular example actually fit on my narrow blog window.
A Better Way?
Well, fixing this type of thing is nice and easy:
private void DoStuff()
{
foreach (thing in thisList)
{
if (!condition1)
{
DoThis(thing);
continue;
}
if (condition2)
{
DoThis(thing);
continue;
}
if (condition3 || condition4 || !condition5 || condition6 || condition7)
{
continue;
}
if (condition8)
{
DoThis(thing);
}
}
}That’s so much prettier. So what’d I do there? A handful of techniques:
- Invert logical blocks if they can reduce your nesting. For condition1, I had an if/else block where DoThis(thing) resided in the bottom else block… farrrrr farrrr away from the check itself. I simply inverted this check and moved the else block up. Of course, I then had to put a continue statement there to go back up to the next iteration.
- For condition2, by simply placing a continue right after the method call in the body, I was able to completely eliminate the else block and reduce nesting by a whole level. This works well with if/else blocks with returns too.
- Next up was a whole pile of combinations for checking when I’m not going to be calling DoThis(thing). That reduced nesting by a bajillion levels, approximately.
- The final block there for condition8 was still necessary. Of course, it could have be written to be the inverse check (so, if NOT condition8) with a continue inside the block, followed by DoThis(thing) outside of the if block. To me this would have been a bit overkill.
Did You Catch That?
- condition1 = true
- conditon2 = false
- conditon3 = false
- condition4 = false
- condition5 = true
- conditon6 = false
- condition7 = false
- condition8 = true
Thus, by combining the condition8 check with the condition1 check, how have I guaranteed all those other conditions? Additionally, how do I know that skipping those condition checks (i.e. pretend they are method calls) has not altered state elsewhere in the class? This optimization actually may not make the code incorrect in certain situations (because it really depends what those conditions are), but it’s important to note that the checks would not be equivalent to the original. It’s just something to pay attention to, but who knows, you may even find that you can optimize some of those checks away depending on your situation!
Summary
