Bug patrol

Automate your software improvement process

Plenty of freely available tools can easily give you reports about what is wrong with your code, thus helping you to improve its quality. And because "wrong" is often subjective, most of the tools can be configured to fit in with your local coding style.

But first it is worth questioning your commitment to reducing bugs. This might sound like a simple question—surely software quality must be a priority for everyone. However, it often seems reasonable to spend minimal time altering code. Any software with a short shelf life should be edited as little as possible, and it is pointless writing a single line of documentation no one will ever read. I wonder how many pieces of documentation have cost more in terms of writing and reviewing than they have saved by somebody reading them? Our aim, therefore, should be to distinguish between jobs worth doing to improve the quality of our work and jobs that are a waste of resources.

How far do you want to go to make your code better?

Several bug categories exist. The tools I look at differ in the types of bug they target. I've identified four different bug categories, which vary in severity from the common or garden bug that annoys users to organizational bugs that prevent code reuse:

  • Current bugs: Current bugs prevent the software from working properly now. They can annoy users and are (or should be) highlighted by your software testing. Current bugs are distinct from latent bugs.
  • Latent bugs: These bugs do not cause problems today. However, they are lurking just waiting to reveal themselves later. The Ariane 5 rocket failure was caused by a float->int conversion error that lay dormant when previous rockets were slower; but the faster Ariane 5 triggered the problem. The Millennium bug is another example of a latent bug that came to light when circumstances changed. Latent bugs are much harder to test using conventional testing techniques, and finding them requires someone with foresight to ask, "What could change that might bring the bugs out from the woodwork?"
  • Accident-waiting-to-happen bugs: Code can be simple and safe, or it can be a dangerous mess waiting to trip up the poor programmers who didn't think long enough about their edits' consequences. Generally, the older the code is, the more likely it is to have sharp, rusty edges; and some industry evidence shows that code not refactored over time ends up requiring a rewrite after an average of seven years.
  • Poorly organized bugs: Poorly organized code is not easily reused and often contains dependencies on more code than necessary. It is consequently more likely to be affected by bugs in other bits of code. Coding standards address this type of problem, but coding standards are not the whole story. They do not stop problems like cyclic dependencies, for example, which kill your chances of maintaining reusable code.

These issues are roughly organized in order of severity. Current bugs are generally more problematic than latent bugs, which are in turn more worrisome than accident-waiting-to-happen or organizational bugs.

The following code contains examples of all of them. Bonus points for spotting them yourself:

1: public static void main(String[] args)
2: {
3:     System.out.println(toUpperCase('q'));
4: }
5:
6: private static char toUpperCase(char main)
7: {
8:     return (char) (main - 31);
9: }

There is a current bug because line 8 should read:

8: return (char) (main - 32);

Without this simple fix, the program won't work. Rather than working out that the upper case of q is Q, it will try to convince us the answer is R.

There is a latent bug because sooner or later someone will try to get the upper case version of ß or of a number and not get anything sensible.

A small accident-waiting-to-happen bug is due to the somewhat silly variable name on line 6. main doesn't really describe what's going on, and is confusing with the method name. Much better code would be:

6: private static char toUpperCase(char lower)

The code would be much safer if it described why it was subtracting 32 from the lower case character.

And finally the code could be better organized if you made the toUpperCase() method public and moved it to a utility class for others to use—better still, you could use the standard Sun Microsystems-provided method:

3: System.out.println(Character.toUpperCase('q'));

Where do you want to stop?

When do you stop tuning and improving your code? There isn't one true answer to this question; the right answer depends on you and your project. Commercial projects often focus on ROI (return on investment) and therefore on fixing current bugs and easy fixes by implementing a coding standard.

It is even common to see commercial projects unwittingly take steps to prevent latent bugs from being fixed by implementing a rigid change control procedure that ties code changes to raised bugs to help testing. It is hard enough to get a team of developers committed enough to care about the quality of someone else's software without putting barriers in their way.

In more enlightened development environments, it is recognized that refactoring is a vital part of quality software production. Refactoring is all about gradually fixing your code's organization to avoid accidents waiting to happen.

But even the most quality-obsessed developers have their limits. Do you really care about punctuation errors in your comments or if the methods are not declared in the most logical order?

Reasoning Inc. recently made headlines by studying the quality of various commercial and open source projects. The company analyzed the source code of the Linux TCP/IP stack, the Apache Web server, and Tomcat, against various commercial alternatives. Interestingly, Reasoning runs its version of the free tools described below and then manually analyzes the results to remove false positives. With some exceptions its results showed that open source's focus on quality above return on investment produced noticeably better quality software.

I identified seven tools that can help highlight defects similar to Reasoning's methodology, and in some cases help fix those defects too. I then used the tools on one of my recent projects called DocTree. DocTree generates a dynamically loading Webpage that contains in-depth links to the Javadocs of tens of thousand classes from many of the important Java software projects. DocTree is not a huge project, but it is large enough to be a useful test case for the seven tools. The source is available from java.net (see Resources), so you can see how the tools are run from Ant on a regular basis.

A few of the tools have graphical user interfaces (GUIs). However, it makes more sense to run them as part of your build process (e.g., from an Ant build script). The less time it takes to run these tools, the more likely that the drive to improve your software will continue.

It is fairly easy to use tools like CruiseControl, Anthill, or even simple cron to fire off nightly builds giving you the results as email. After setting up such a procedure, it becomes easier to use than a GUI.

Let's get into the seven tools. I will cover:

  • FindBugs
  • PMD/CPD
  • Checkstyle
  • Jalopy/Jacobe
  • JDepend
  • JUnit
  • Eclipse

FindBugs

FindBugs focuses on current and latent bugs. It looks for common issues and bits of code that often indicate something is wrong.

Some example issues highlighted by FindBugs include:

  • Methods that return mutable static data
  • Get and set methods where the get is unsynchronized while the set is synchronized
  • Problems in how Iterator classes are defined
  • Control flow statements that have no effect
  • Places where a NullPointerException might occur and places where redundant comparisons of reference values against null are made

FindBugs is under very active development, and you can extend it via a plug-in mechanism to include customized checks.

FindBugs found one fairly serious latent bug in DocTree: a class that defined equals() but not hashcode(). This didn't cause any problems in the current configuration. However, a future DocTree iteration that used the class in a Hashtable or HashMap could find spurious results. FindBugs also found some minor latent bugs like streams left unclosed on an exception until garbage collection and other minor problems. As the number of highlighted issues goes up, the number of bugs in your software should go down.

FindBugs uses byte-code analysis to generate its fault reports.

This is a DocTree project snippet of an Ant target to create a FindBugs report:

<target name="web.findbugs" depends="jar">
<mkdir dir="${target.web}/findbugs"/>
<taskdef name="findbugs"
    classname="edu.umd.cs.findbugs.anttask.FindBugsTask">
  <classpath>
    <fileset dir="${support.tools}/findbugs066"
        includes="**/*.jar"/>
  </classpath>
</taskdef>
<findbugs home="${support.tools}/findbugs066"
    output="text"
    outputFile="${target.web}/findbugs/report.txt"
    reportLevel="low" sort="text">
  <class location="${target.jar}/${ant.project.name}.jar"/>
  <sourcePath path="${source.java}/main"/>
</findbugs>
</target>

PMD/CPD

PMD is very customizable and can find issues in all four bug categories. However, it is strongest in finding organizational and accident-waiting-to-happen bugs. Unlike FindBugs, PMD uses source code analysis and works similarly to a compiler, except it generates bug reports rather than byte code. PMD is one of the most customizable tools I examined for this article mainly because of a cool feature that allows you to access the source code parser's output using an XPath engine. The parser generates a tree in which the nodes are the parts of your program. So a class node will contain child nodes for each variable and method. The method nodes will then contain child nodes for the statements within that method. PMD allows you to access this tree as if it were an XML document using XPath.

So once you understand XPath and some basics about how the parser generates the node tree from your source code, you can easily add new rules. For example, the following expression from the PMD documentation demonstrates how to check that all while statements use curly brackets:

    //WhileStatement[not(Statement/Block)]

This looks for while statements anywhere in the tree (//) that do not contain a Block (parser terminology for some curly brackets).

Some example issues detected by PMD include:

  • Empty blocks
  • Excessive parameter counts for methods
  • Excessively long classes or long import lists
  • Unused fields, methods, and variables
  • Broken double-checked locking

Some of these checks are very subjective. One person's nicely encapsulated component is another person's excessively long class. So, thankfully you can configure PMD to check for the issues you consider important. The ability to skip checks is particularly crucial when you use PMD for the first time on larger projects. If you run the full set of checks from scratch, PMD will generate several years' worth of fixes, so it's beneficial to start with the more important checks.

CPD (Copy/Paste Detector) is a related utility that detects copy and paste code within a project. Large amounts of duplicated code can indicate that instead of using inheritance or creating libraries a developer has found similar code and copied everything, bugs and all.

The PMD Ant target from the DocTree project looks like this:

<target name="web.pmd">
  <taskdef name="pmd" classname="net.sourceforge.pmd.ant.PMDTask">
    <classpath>
      <fileset dir="${support.tools}/pmd121" includes="**/*.jar"/>
    </classpath>
  </taskdef>
  <mkdir dir="${target.web}/pmd"/>
  <pmd rulesetfiles="${basedir}/${support.tools}/pmd121/ruleset.xml"
      shortFilenames="true">
    <formatter type="html" toFile="${target.web}/pmd/index.html"/>
    <fileset dir="${source.java}/main" includes="**/*.java"/>
  </pmd>
</target>

Checkstyle

Checkstyle is fairly similar to PMD although it is more tuned to finding organizational bugs. Like PMD it uses a compiler front end to generate its bug reports. PMD provides many more checks over a wider range of areas. However, Checkstyle is more configurable. And while some of the checks overlap, many of Checkstyle's checks are not in PMD and vice versa. For most people it will be enough to pick one or the other.

Checkstyle can find:

  • Unused or redundant imports
  • Use of tabs where spaces are preferred (or visa versa)
  • Variables, methods, or classes not adhering to a naming standard
  • Overly complex assignments or return statements

The Checkstyle Ant target from DocTree looks like this:

1 2 Page 1
Page 1 of 2