This post originated from an RSS feed registered with Java Buzz
by Elliotte Rusty Harold.
Original Post: Prefer Multiline if
Feed Title: The Cafes
Feed URL: http://cafe.elharo.com/feed/atom/?
Feed Description: Longer than a blog; shorter than a book
C-family languages including Java, C#, and C++ do not require braces around single line blocks. For example, this is a legal loop:
for (int i=0; i < args.length; i++) process(args[i]);
So’s this:
for (int i=0; i < args.length; i++)
process(args[i]);
However both of these are very bad form, and lead to buggy code. All blocks in C-like languages should be explicitly delimited by braces across multiple lines in all cases. Here’s why:
The most dangerous form is a multiline block that doesn’t use braces. The problem is that when you start with this:
if (p.needsGiftWrapping())
wrap(p);
sooner or later some programmer–perhaps you, perhaps someone else–is going to discover a need to add a second line. For example:
if (p.needsGiftWrapping())
wrap(p);
ribbon(p);
Bang! That code is now buggy. It looks like the if block applies to both statements, but in fact it only applies to one. The indentation is lying about the intent of the code. By contrast, this form is less dangerous:
if (p.needsGiftWrapping()) wrap(p);
In this example, it’s obvious to any programmer who comes along and adds a line, that they need to add braces too. It’s less likely to cause bugs down the road, but it should still be avoided, and here’s why.
The statement
if (p.needsGiftWrapping()) wrap(p);
is really two statements: one that calls needsGiftWrapping() and one that calls wrap(p). These are independent statements, and may need to eb treated separately. In particular, you may want to mark a breakpoint on one and not the other. For instance, I sometimes like to put a breakpoint in the body of an if or for or while, just to make sure that this code is really being executed when I think it is. If the program doesn’t stop, then the code isn’t being executed; and I have a big clue where the bug is.
This is also important for code coverage tools. Most tools such as Cobertura measure the coverage of lines of code, not statements. Even if they measure lines of code coverage and statement coverage separately (and some tools do this) they still display the coverage with lines of code. This statement can be marked covered even if the body of the block is never entered:
if (p.needsGiftWrapping()) wrap(p);
However, if the block is rewritten like this:
if (p.needsGiftWrapping()) {
wrap(p);
}
it now becomes obvious if the tests are never testing the case where p.needsGiftWrapping() returns true.
This applies equally to all block statements: if, for, while, do while, and any others you may encounter. I’m beginning to believe this is actually a special case of a general principle for C-like languages, and perhaps others:
Each line of source should contain exactly one statement
That is, avoid lines like this:
int i = 7, j = 18;
or
int i = j = 18;
Similarly, avoid the ?: operator.
if (a > b) {
max = a;
}
else {
max = b;
}
is easier to read than
max = a < b ? a : b;
For instance, did you even notice the bug in the above line? If the verbosity bothers you, try something like this instead:
max = Math.max(a, b);
The problem may reflect a misdesign in C-family languages. The compiler only pays attention to the semicolons and braces while ignoring the line breaks and indentation, but humans usually only pay attention to the line breaks and indentation while ignoring the semicolons and braces. This gives the code the opportunity to lie about what it’s really doing. Consequently we need to take extra care when writing in C, Java, C++, C#, etc. not to lie to ourselves. If you place exactly one statement on each source line, you can be reasonably confident the code isn;t lying to you, and you’ll have a much easier time debugging.
Compact code is fun, but it’s not maximally readable, and more lines don’t really cost you anything after the compiler is finished anyway. Save yourself the hassle and for 2009 resolve to put one statement on each line.