Saturday, July 27, 2013

Killa: the case for mandatory braces

Killa was created following the Perl/Ruby philosophy of freedom of choice. So you must be wondering why on earth I decided to add the rule of mandatory braces for blocks. Its a valid question, one that I would try to answer in this post. But first a bit of history.

My first approach to programming was at the university. The career I was following (electrical engineering) was not centered about programming, so programming was for us a mere exercise of getting additional credits. I don't remember much about my time at the university, but what I can not forget is our "code" and I mean the spaghetti-like thing that most of us produced for code: horrendous single file programs without proper naming conventions neither format with piles and piles of commented sections that for some miraculous reason worked enough to pass the tests at hand. I suspect that the pain of working in that kind of environment was the main reason behind me starting to use my own set of code conventions, and even when my programming style has changed a lot through the years, the first rule has always been the same: never ever forget to use braces for code blocks. No excuses.

So lets see some code. In many C-derived languages (like C++, C#, Java, JavaScript), braces are optional. So this code:

    if ( enemyIsNear() ) {
        launchTheMissiles();
    }

can be written like this:

    if ( enemyIsNear() )
        launchTheMissiles();

And this it's perfectly legal and used a lot on industrial-grade software.


You could be wondering (with reason) What is wrong with this? And the answer is nothing... for the time being. The missiles are launched only when enemies are near. But as Joel Spolsky would say, this code just smells a bit bad. There is nothing preventing another programmer to add a line of code to the conditional and forgetting to add the required braces. Imagine we need to speed up before launching the missiles, we could do this:

    if ( enemyIsNear() )
        speedUp();
        launchTheMissiles();

but now the missiles are launched even if we are not near an enemy! Because the above snippet is equivalent to:

    if ( enemyIsNear() ) {
        speedUp();
    }
    launchTheMissiles();

A total waste of missiles just because we were saving a pair of braces! Clearly this is not good, but lets see some arguments that are used against mandatory braces.

The code compressors

Some professional programmers could favor optional braces under the argument that forcing braces adds one or two more lines of code in blocks like the one presented at the beginning. After all, under some code conventions the original snippet ends like this:

    if ( enemyIsNear() )
    {
        launchTheMissiles();
    }

Four lines instead of two! A total waste of space! Considering all the cases where additional new lines are introduced we obviously get more lengthy programs.

Less code is better, they'll say. OK, sure! But if programming were a simple matter of removing new lines this would be even better:

    if ( enemyIsNear() ) speedUp();

You could be wondering: What is wrong now with this line of code?
And I'd say ...again nothing! Until you want to test your program and check if the conditional is working in all the cases that it must work. Some kind of code coverage. The program hitting that line doesn't give you any indication about the conditional being passed or not. For this example we could check if the function was called, but that wouldn't be the case if the code were like this:

    if ( enemyIsNear() ) speed = 2 * speed;

it is not clear how can we validate if the speed is being doubled or not. Adding a breakpoint to this line for debugging is going to stop the program always, so you'll need to break the line anyways for debugging purposes.

So the argument of using less lines of code is moot, less code is better code but only if the resulting code does the same, has the same number of errors and maintains the same quality. If this not were the case, the code minifiers would be the best coders ever. However, still we need to probe that using mandatory braces produces code of better quality.

The indentation believers

Some professional programmers would believe that correct indentation is good enough to indicate the beginning and end of code blocks. Indeed, there are some languages that go one step ahead and remove the need for block delimiters, indentation is used for block scoping. In Python the original snippet looks like this:

    if enemyIsNear():
        launchTheMissiles()

No need to argue about mandatory braces here, there are not even braces now. Cool. Everything is good and well but someday you realize that you need to check the engine heat after launching the missiles, so you do:

    if enemyIsNear():
        launchTheMissiles()
        checkEngineHeat()

The engine check is a time wasting process but it's fine because it's done only after we launched the missiles. And then someday another engineer realizes that for this case in particular the check is always required, doing:

    if enemyIsNear():
        launchTheMissiles()
    checkEngineHeat()

Just looking at this change, I tell you, I'll freak out. What if for some reason the guy accidentally deleted my precious tab?. Nope, we don't use tabs here. But What if he thought I was wrong and I did the original change wrong and now he fixed it. I won't rest happy until having a little chat with the cunning fellow. Of course if this where a industrial-grade software, this change alone would come with a lengthy and detailed description in the commit log, or even better with profuse comments on the program itself. But my point stands, format alone is not enough to clearly state program intention. Indentation is a decoration, it's a luxury, and you must be free to decorate your programs the way you like. Just leave my block delimiters alone. The other problem with using indentation as block delimiters is that indentation is done using  withespace. And whitespace is by definition not visible. Yes, you could force whitespace to be visible in your editor, but that is forcing and it is at the very least discussable. I mean, if it were good enough we all must be programming in Whitespace for the fun. A side benefit of not using indentation for block delimiters is that you can safely ignore any formatting change in code reviews: just ignore whitespace. I won't talk about the issues of copy pasting code in Python, copy pasting code is bad anyways and you deserve to be toasted.

That said, I don't think I'll convert many believers with my arguments here: they are believers for some reason and I'm just a non believer. For them code like this:

    if ( enemyIsNear() ) {
        launchTheMissiles();
        checkEngineHeat();
    }

it's just tiresome and bloated source code.

The C pitfall

C is a great language, but it could have been even greater. One of it's design fails came when it allowed optional block delimiters. Consider this classical example:

    if ( enemyIsNear() )
        if ( isWeakEnemy() )
            launchTheMissiles();
    else
        watchTV();


You could be thinking that it's safe to watch TV if there are not enemies near. But what this code really is doing is:

    if ( enemyIsNear() ) {
        if ( isWeakEnemy() ) {
            launchTheMissiles();
        } else {
            watchTV();

        }
    }

Yeah, you would be watching TV with a mighty enemy on your tail. Scary, isn't? C doesn't use indentation for block scope, but it allows optional block delimiters. Bad things could happen, and now you are falling on your parachute looking the ground under your feet approach.

About the possible bug with which we started this post, to be honest, I have rarely seen it in production code, even less in committed code. I can't introduce it because I use mandatory braces, my team can't introduce it, because I established the company guidelines with mandatory braces on it and most of the projects I have worked on use a code convention that forces mandatory braces or are careful enough to avoid this class of blunder even if they are not using mandatory braces. Or that I though. But upgrading Killa with the fixes in the Lua 5.2.2 release (Killa used the Lua 5.2 version) show me some remarkable fix in the line 824 of lapi.c:



Looks familiar? Yes. The scary thing is that this bug was able to hide itself for more than one year in a project reviewed by hundreds of savvy programmers (me included) but we totally missed it. When working in Killa I had two options: reformat all the source code to use my code standard (mandatory braces), or leave it like it was. I was in a hurry and decided for the second option, also reformatting the source code would imply a lot more of work back-porting upstream changes. The sad thing is that manual reformatting would have surely made me question the code in mention, the indentation was wrong at the very least.

For the sake of redemption, The Go language developed by Ken Thompson (the father of the B language from which C was derived) uses now mandatory braces.

Conclusion

I argued against two camps that oppose mandatory braces and showed some pitfalls about allowing optional braces in legacy languages, but at the end, I failed on giving you hard evidence about this, I don't have the statistical numbers at hand, just some anecdotal evidence and you are free to think it's not enough.

The reason I implemented mandatory braces in Killa is that I believe this has the benefit of making clearly evident the structure and intention of the underlying programs. It's not fool safe but a lot better than leaving free choice in this regard.

Freedom, there is still plenty: you can use tabs if you prefer them, 2 spaces or 8 spaces? it's for you to decide, the Lisp style is your thing? be my guest. Just leave my braces alone.

No comments:

Post a Comment