ALTERthought Blogs

14 March 2006

Better Code Automagically, Part 1: FindBugs

This post is intended to be first in a series (maybe) of discussions around mechanisms for increasing code-quality thru the use of automated processes, such as:

The grail here is to explore approaches that can not only increase code-quality but can simultaneously decrease the amount of time that the senior developers need to spend in code reviews and Q&A. (It is recognized that few things can take the place of a good-old-fashioned code review, but time often prohibits such undertakings on a busy personnel strapped project.)

I thought I would start with a discussion of a tool that until recently I was only superficially aware of: FindBugs

FindBugs is an OpenSource tool that falls into the Metric and Code auditing camp, and in that sense it operates in a fashion similar to such products as CheckStyle:

  1. Point FindBugs at a code base using Ant (there is an Eclipse plugin as well)
  2. Have it analyze your code and produce a report on what it found

However, unlike CheckStyle (Which, BTW, I consider indispensable, and will likely write about in another post) FindBugs operates on the compiled java ByteCode not your source code. This is interesting, in that it often ferrets out potential bugs that other tools miss completely….

Here is an example of the type of legitimate bug that FindBug can uncover. Picture the following ‘real-world’ (if a tad mondane) scenario:

public class Moose {
private Integer counter = 0;
public synchronized incrementCounter() {
counter++;
}

public synchronized decrementCounter() {
counter--;
}

public reset () {
counter = 0;
}

public doit(boolean isDone) {
if ( isDone ) {
reset();
} else if ( counter < 100 ) {
incrementCounter();
} else {
decrementCounter();
}
}
}

OK,take a minute. Do you see what the problem is? FindBugs does. It reports (answer italicized below):

Inconsistent synchronization
The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that
- The class contains a mix of locked and unlocked accesses,
- At least one locked access was performed by one of the class’s own methods, and
- The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

The problem in question was that the reset() method was not synchronized. huh.

An addition to legitimate ‘this-might-break-your-application’ type errors, FindBugs can help make your code more performant. The following code:

String buffer= "";
for (int i=0; i < 10; i++) {
buffer = buffer + ":";
}

Results in FindBugs reporting:

1 SBSC: Method com.alterthought.Sample.f() concatenates strings using + in a loop

..and gives a more detailed explanation of:

Method concatenates strings using + in a loop
The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted
back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.
Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 1.5) explicitly.

So is FindBugs a panacea? a mircle tool? the silver bullet? Alas, of course, not. The eclipse plugin proved buggy; often reporting misaligned line numbers (it appears to be thrown off by Java5 annotations)...and muc of what FindBugs reports can be had in a more convienent package by way of CheckStyle or even the Eclipse IDE. Regardless, incorporating FindBugs into your build paradigm is easy enough to do; the price is right (free) -- and if it finds a single bug its likely paid for the effort.

Technorati Tags: , , ,

Post to Twitter Post to Delicious Post to Digg Post to Reddit

Comments are locked.