Richard has posted 3 posts at DZone. View Full User Profile

Omitting Braces: Not Just A Matter Of Style

07.30.2008
| 15317 views |
  • submit to reddit
According to our analysis of more than 100 Java projects, one of the most violated rules from static analysis, is 'missing braces in If statement'. Although often considered to be merely a stylistic preference, we found that omitting braces is actually a good indicator of probable buggy code.

For example, when developers need to add to existing code and forget to add braces where the original statement only contained a single line of code, e.g.


//original code
if (condition)
doSomething();

//new code
if (condition)
doSomething();
doSomethingElse();

The call to 'doSomethingElse()' is only meant to execute if 'condition' is true. However, omitting the braces in the If statement means doSomethingElse() will execute regardless of t he value of 'condition', hence the bug.

It may seem obvious but after that large lunch on a Friday.....who knows ;-)

From our individual metrics studies we found the Halstead Program Volume was the best individual metric for finding files that might contain bugs.

This means that, if you know nothing else about the file, a high Program Volume metric means the file is more likely to contain a bug. Our ongoing research indicates that a Program Volume value of 4000 reflects a 50% likelihood that a bug is present in the code. A Value of 5790 increases the likelihood to 90%.

For more details on this and other metrics that are indicators of possibly buggy code read our technical paper.
Published at DZone with permission of its author, Richard Sharpe.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Ritesh Chitlangi replied on Wed, 2008/07/30 - 7:35am

You should submit a JSR for it.

Jess Holle replied on Wed, 2008/07/30 - 7:45am

Braces are a matter of style -- I never add them unless they're needed.  Yet in over a decade of coding I can't recall ever having made the error shown.  Perhaps I did while I was still in school, but that was a long time ago.

Software is a field for the obsessively compulsive.  If you don't have a little OCD in your own internal code review practices, you'll likely have a lot more issues downstream.

George Jempty replied on Wed, 2008/07/30 - 8:13am

Code without braces is also harder to maintain.  If the maintenance developer needs to add a statement, they also need to add the braces.  If they miss the fact that you didn't use braces, then they will be introducing bugs.  I am thankful that my first "braced" language was Perl, where the braces are mandatory.  It got me in the habit, and there's been no good reason to break that habit.

Mohamed El-beltagy replied on Wed, 2008/07/30 - 9:19am

I totally agree. The mentioned code is common place for bugs.

Mileta Cekovic replied on Wed, 2008/07/30 - 9:23am in response to: Jess Holle

[quote=jessh]

Braces are a matter of style -- I never add them unless they're needed.  Yet in over a decade of coding I can't recall ever having made the error shown.  Perhaps I did while I was still in school, but that was a long time ago.

Software is a field for the obsessively compulsive.  If you don't have a little OCD in your own internal code review practices, you'll likely have a lot more issues downstream.

[/quote]

I have exactly the same experience. Adding braces in this case violates YAGNI principle as well as it occupies one line of the precious screen space. The more code you see, the better overal overview of the code you have. And this has nothing to do with the screen resolution. I've worked in IDEs from 800x600 to 1680x1050 and I am still lacking space (that is why in my organization almost all developers have two monitors).

My personal rule for adding braces is to put braces if and only if the block that braces should enclose occupies more then one screen line, regardless of the number of statements in the block. For example if you have nested 'ifs', you  would put braces on outer if, and not on inner, even if there are no more statements in outer if other then inner if.

Regarding code maintainers adding lines and forget to add braces: well, code maintainers can make much worse errors if they do not 'know' the code. They have to get familiar with the code they mainintain or they will not maintain it correctly. And if/when they get familiar, they will not make such kind of mistakes ;).

For example, just recently I had to fix a bug in the clustering support in the client library (that was written in year 2000) of the legacy MOM middleware in my organisation. I ended refactoring the whole client library and rewriting the clustering support in 3 days. So of you have to maintain some old code, it is recommended to first refactor it to get familiar with the code, and then you will be able to change sensitive parts too (like clustering support in my case).

For  the end, if most pople do not use this kind of static analysis error checking then it probably means something: i.e. that this makes more harm then is usefull.

David Karr replied on Wed, 2008/07/30 - 10:36am

I've been designing and writing software for almost 30 years now, and one important thing I've learned about coding and programming guidelines is that if you can't clearly articulate WHY you should follow a particular guideline, or if you can't see the possible tradeoffs of that guideline, then you should probably think harder about why you should do this thing.

To this continuing argument about whether to add braces around single-line blocks, I would add the following points:

1. Code is read much, much, more often than it is written. Guidelines that make code more readable and concise, towards the goal of being able to see an entire cohesive block of code on a single screen, are very worthwhile, and guidelines that work against that goal are less worthwhile.

2. The vast majority of programmers, even the experienced ones, are writing code in an IDE that has an "intelligent" tab (and return) key that knows to indent to the column implied by the context of the current line. If you press Tab on the line after a single-line block, it will NOT indent to the column under the previous line, but the column just before it. If that happens, and you intended the new line to be part of that presently single-line block, then it will be completely obvious to you how to rectify that.

I learned to ignore the "avoid single-line unbraced blocks" guideline many years ago, and I've never made the mistake many people attribute to the problem this guideline purports to resolve.

Raveman Ravemanus replied on Wed, 2008/07/30 - 10:40am

The question is how important is it ? many people are mad when class starts with small letter or method with big letter(its common in other languages). However many people are in love with final parameters for method or with using formatter (many times it makes code look bad, but we do it anyway. ugly, but formatted code). The real question is should we do it?

Oleg Sobolev replied on Wed, 2008/07/30 - 10:38am

I think it's ok not to put braces around 'flow control' statements - single return/throw/break/continue statements. If you later add more code either before or after these statements forgetting about braces, compiler will complain about unreachable code.

Thomas Angioletti replied on Wed, 2008/07/30 - 12:00pm

Personally I prefer no braces only when the code will fit on the same line as the if statement:

if(x==1) x=4;

I wouldn't use:

if(x==1)

x=4;

for the bug potential, and I would prefer not to use

 if(x==1)

{

    x=4;

}

because it's too big.  

 

Brian Sayatovic replied on Wed, 2008/07/30 - 12:00pm in response to: Oleg Sobolev

I completely agree.

I never write a non-braced if statement on multiple lines, but keep it on one.  This makes short-circuit fail-fast checks very succinct and consumes little screen real-estate:

if(!precondition) throw new IllegalStateException(...)

This is not unlike usingthe assert keyword.  Yet when the reaction to a condition becomes too complex, I'll revert to braces and multiple lines (or, better yet, use a private helper method to keep the if statement itself readable in the context in which it exists).

Rich Bagley replied on Wed, 2008/07/30 - 3:58pm

I agree with the author.  I've come across this bug a number of times and what makes it so tricky is that the mind perceives the structure of the program section wrongly.  And the more you look at it, the better it looks.

 Consistency is the key to creating creating code that is readable and maintainable.  I think maybe fourth or fifth considereration is how many lines or keystrokes are used.

"Programs must be written for people to read, and only incidentally for machines to execute."
- Abelson, Sussman and Sussman, Structure and Interpretation of Computer Programs, preface to the first edition

Steven Baker replied on Wed, 2008/07/30 - 6:04pm

i used to code like thomas and brian... but then i was coached by a pretty good senior developer, and have not gone back from the full bracer expressions...

 

any time i now see a lack of bracers, i immediately think: just out of uni newb

Krystan Honour replied on Thu, 2008/07/31 - 5:07am

To be honest I can see the pros and cons of adding and not adding braces to single line if statements. However I've always added them, the reason is I want to see where a block of code starts and ends.

 As for code like this

public int foo(final SomeObject so) {
    if (so.getSomeValue() == INVALID)
        return FAILED;
    else
        return SUCCESS;
} 

well the non single point of return is a whole other holy war which we won't restart here but I don't beleive this practise can be used to justify the non adding of braces in any case.. to me consistent style accross code is important and that means every if statement conforming to the same standard (with braces).

 to my mind creating blocks of identifiable code be it one line or 10 is worth the extra two lines and that is why I follow that "rule".
 

Paul Davis replied on Thu, 2008/07/31 - 11:52am in response to: Krystan Honour

Why not

    return (so.getSomeValue() != INVALID);

 

<pre>:-)</pre>

Krystan Honour replied on Thu, 2008/07/31 - 4:18pm

No reason (i note the smiley)

 It was just the easiest way to demonstrate the point I suppose i should have put "...." above the test haha

Richard Sharpe replied on Fri, 2008/08/01 - 9:49am

Some really interesting comments - thanks.

If you omit braces and use Static Analysis tools it is aproblem actually finding these bugs. Why? Because you probably have the ruleturned off!

Having any tool tell you of violations in your code purelydue to a stylistic preference will result in thousands of false positives andeventually demotivate the developer from using the tool, therefore, for theother good causes these tools promote, in this case it is probably better toswitch the rule off. 

Hopefully Unit Tests are in place to cover these areas ifstatic analysis is not used. The only other way to find these bugs is manualcode review (laborious, time consuming and introduces human error i.e. the bugmay be missed anyway) or debugger tracing. 

Many of those who omit braces in If Statements do so for readability purposes.Psychologically, this may mean that these developers see readability as ahigher aspect of quality than possiblefault-prone code. I’m not stating that readability is not important, far fromit! However from a business perspective, having the code released with lessfaults is a higher quality perspective.

Kookee Gacho replied on Tue, 2012/05/29 - 6:24am

Because Java is both the name of a programming language and the name of the program, also called platform, used to develop and/or run programs written in the Java programming language.-Arthur van der Vant

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.