Braceless if considered harmful
There are some things I’m reluctant to write about because everything that needs to be said about them has long since been said. This post falls into that category. Peter van der Linden explained what I’m about to say in Expert C Programming: Deep C Secrets years before Java was released, and I have nothing to add to what he wrote. Nonetheless very few C or Java programmers got his message so it’s worth saying again, and repeating until the community finally learns.
Always use braces on multiline if statements
Code like the following is syntactically correct; but it is bug-prone, and should be rewritten:
if (x > 3)
doSomething(x);
// continue with rest of program...The problem is sooner or later you or someone else is going to need add a line to that if statement, and you’ll end up with something like this:
if (x > 3)
y = x - 3;
doSomething(x);
// continue with rest of program...The program has suddenly become buggy. Do you see the bug? Not everyone does, especially under deadline pressure when the buggy lines aren’t called out and pointed to as they are here. Even written like this, not everyone will see the bug immediately. The problem is the call to doSomething(x) is no longer conditioned by x > 3. The indentation reflects the intent, but the indentation is not significant to the compiler. What the compiler sees is this:
if (x > 3)
y = x - 3;
doSomething(x);
// continue with rest of program...Of course, I should have written the revised code like this instead:
if (x > 3) {
y = x - 3;
doSomething(x);
}
// continue with rest of program...However in the heat of programming, especially maintenance programming, it’s really easy to miss this. Worse yet, this bug may not immediately be apparent. It’s possible that the code will still continue to run and execute correctly most of the time. Here that would be the case if x were greater than 3 much more often than it was less than three. (Sidebar: religious use of a code coverage tool, especially one that checks boundary conditions, might well have exposed this bug. However that would require close to 100% coverage, a level few projects reach.)
It’s a nasty bug that can be hard to find. However it can be completely eliminated by the simple expedient of always using braces for multiline if blocks, even if blocks that only contain a single statement. For example, the original code should have been written like this:
if (x > 3) {
doSomething(x);
}
// continue with rest of program...or like this:
if (x > 3) doSomething(x);
// continue with rest of program...In the latter case it ‘s obvious to the maintenance programmer who needs to add a statement to the block that they also have to add braces. With the former, the braces are already there so they’ll almost certainly add the code inside the braces. But without braces and with the block crossing more than one line, there’s only about a 50-50 chance the maintenance programmer will notice they have to add braces too. Those aren’t odds I’m comfortable with.
Be kind to the programmers who come after you. Always use braces in multiline if statements.
P.S. Of course the same is true for for, while, and foreach blocks. However, in practice if blocks seem to account for the lion’s share of these sorts of bugs. Single statement for, while, and foreach blocks just aren’t very common so they almost always have braces.
January 23rd, 2006 at 10:44 pm
Of course, in the braceless, indentation-loving world of Python, this particular error is completely impossible….
January 27th, 2006 at 5:41 am
I totally agree with this rule, trying to enforce it in all my work.
At worse, if really one want to be concise, one can write:
if (bFoo) break;
Having the instruction on the same line may help avoiding the described pitfall.
Note that this can be enforced. See, for example, http://pmd.sourceforge.net/rules/braces.html
February 15th, 2006 at 11:06 am
In Perl, braces are required in ifs and whiles and such, so I’ve now gotten in the habit of always using them in C and Java as well, except (as noted above) when the controlled statement is on the same line. This is done in Perl as “foo() if bar;”, so I feel justified in using “if (bar) foo();”
February 21st, 2006 at 4:18 pm
This has been part of our Java coding standards since I wrote them in 1999.
February 23rd, 2006 at 8:10 am
It’s also a justifiable reason for always putting { on the end of the line, and } else { on the same line. Arguing about aesthetics is one thing (whichever ‘reads’ better to you is the one you’ve been using the most) but putting } and { on the same line as the if/while/for/else/try/catch is the only way to prevent errors by insertion of a line of code. It doesn’t even have to do anything; a System.out.println() will break code if put in the wrong place.
Alex.
March 2nd, 2006 at 4:19 pm
I don’t understand what cuddled elses (putting } on the same line as else/catch) has to do with safety. Can someone explain that?
September 14th, 2006 at 11:09 am
if (x>3)
####dosomething();
Now if I want to add a line of code:
if (x>3)
{
####dosomething();
####dosomethingelse();
}
(used # instead of spaces in the case this form would cut blank space)
You see? I know that not using {} only works for one line. The fact that I used braceless if in the first time is proof that I already know about this. And I also know that the compiler ignores whitespace so if:
if (x>3)
####dosomething();
####dosomethingelse();
will always run dosomethingelse() IT IS SUPPOSED TO WORK THAT WAY. You see? I am coding in JAVA/ C++ / C# I am sure that I am not coding in python, so I can’t expect that if to cover both statements just because of the identation.
if (x>3)
{
####dosomething();
}
Is simply an excesive usage of space for a single-line if. In my opinion, others may argue that but that’s the good thing about this world, I can use that syntax, I am not using a condescendant compiler that expects me to make silly mistakes.
But in my opinion the best would be if (x>3) dosomething() ; It is logical, it is short and prevents the first mistake.
“However in the heat of programming, especially maintenance programming, it’s really easy to miss this”
September 23rd, 2006 at 2:32 pm
> will always run dosomethingelse() IT IS SUPPOSED TO WORK THAT WAY. You see?
The point of the post is that 95% of the time it isn’t supposed to work that way, and it’s a bug.
And when it’s supposed to work that way and isn’t a bug, then it’s sheer stupidity, because most programmer’s eyes are trained to check the fastest way to see blocks: check indentation.
Therefore most people will think that dosomethingelse() is inside the if block, and will misunderstand the code unless they take a second look at the exact code.
Therefore, that kind of code is either a “software bug” (the software will be buggy) or a “programmer bug” (the programmer won’t get the code, and will be buggy).
> if (x>3)
> {
> ####dosomething();
> }
> Is simply an excesive usage of space for a single-line if.
And this is why Kernighan and Ritchie invented the K&R style
if (x>3) {
doSomething();
}
Look ‘ma, no excessive use of space even for single-line conditionals!
November 13th, 2006 at 1:24 pm
[…] Smart developers have known for years that braceless multiline statements are bug prone. The problem is that the indentation can be actively misleading and cause bugs during code evolution and maintenance. For example, suppose you start with this if statement: […]
December 6th, 2006 at 6:37 pm
The problems described above would not happen in a Test Driven Development environment (typically XP shops).
If I’m not attentive and forget my braces, I’ll discover my error in the next few minutes.
“What if someone adds another line and forgets to add the braces?” – This is fear-based programming and has no place in confident, courageous coding.
June 14th, 2007 at 3:58 am
Anyone who can’t immediately see the error in the example with two indented statements without braces is broken; such people are bug-prone in any case, and lord only knows what other blatant bugs they’re missing because they are incapable of reading the language. But even such broken people can avoid this sort of bug simply by using the right tools — use an auto-indenting editor and such incidents simply can’t happen. That’s a far better crutch for those who need one, especially since such broken people are liable to forget to add the required braces — if they can’t be bothered to notice something as obvious as that they’re adding a second statement to an if block without braces, then they can’t be counted on to add the extra braces. If I had a maintenance programmer who was that sloppy, I would transfer them immediately to QA where they could experience the consequences of such behavior.
November 8th, 2007 at 11:44 pm
I agree with truth machine. In 20+ years of programming (C, C++, Lisp, Java, Perl and now PHP…) we never met the problem (bug),
if (x)
y();
z();
The code should be indented with the right tool anyway (e.g. Emacs). A good programmer doesn’t fall in such a “trap”.
In Perl when there are a lot of consecutive if-else[if] statement, the code is full of {}s even for single statements – it is harder to read.
if (a) { while (b) { if (c) { for (;d;) { e(); } } } } ??As for the
// A
if (x) {
y();
z();
}
// vs B
if (x)
{
y();
z();
}
I go for B. Someone will inject a line between the “if” and the “{“? and let the {}s alone??
When there is a lot of nested conditions/while/for… the code is well spaced out, and readability is increased.
January 18th, 2008 at 5:07 pm
Personally, I like to have simple single line if statements without the braces. I did it the other way for years until I discovered that braces weren’t required. (Learned it from the source library for the standard Java classes.) To me, the code is just much prettier – cleaner looking – without the braces and I, personally, have never run into any of the issues that have been described. However, I can certainly imagine that some might have trouble following the braces, or lack thereof. For me it has worked and the aesthetic satisfaction I get from looking at the code produced weighs in significantly against the potential bugs that I have never experienced.
I also like to keep my conditions separate from the code that depends on them as I find it more readable that way. (Especially when I have a moderately length condition or longer.)
Certainly this could be different in a collaborative environment.
March 28th, 2009 at 5:05 pm
If you’re dumba$$ enough to not notice the bug in this example IMMEDIATELY, you need to switch professions fast, not ruin others’ code with your overly verbose, ugly crutch.
if (x > 3)
__y = x – 3;
__doSomething(x);
// continue with rest of program…
February 6th, 2014 at 3:47 am
[…] Braceless if considered harmful (cafe.elharo.com/java/braceless-if-considered-harmful/) […]