Daily Archives: 26/03/2010

Taking a look at FindBugs in a real project: checking DirectJNgine

I have been testing FindBugs effectiveness and ease of use in an existing project, DirectJNgine. A small project, 5.600 Java lines, plus 2.000 Javascript lines.

First conclusions

With all settings set to the maximum strictness, FindBugs reported 70 issues. Do not get alarmed, though! Issues are things FindBugs thinks you should check, not necessarily bugs.

In fact, only one of these issues ended up being a bug, one that would happen once in a lifetime -but it was a real bug.

The experience was very positive: in fact, I never found a diagnostic I disagreed with. And, in a few cases, FindBugs pinpointed code that I just did not know could be suspicious.

In many cases, FindBugs reported performance issues that, I knew beforehand were absolutely irrelevant, but would have been a serious problem in a different context.

That said I converted all code to the better performing version, if only to make FindBugs happy.

Now, this is very important: I strongly believe you should never commit code with outstanding warnings or issues to your version control system. This destroys credibility and trust.

Another way to say this is do not live with broken windows… Yes, sometimes you will have to do some extra work just to please FindBugs, but I think this will pay off later.

Back to FindBugs, in 80% of the cases the diagnostic explanation allowed me to “fix” the code directly.

When that was not the case,  it provided me sufficient information to let me find the way to fix the problem with ease.

To be fair, the only “poor diagnostic” case I found was for circular class dependencies!

Of all checks performed by FindBugs, I disabled only the checks for circular class dependencies (two cases), and only because FindBugs is a stable and mature project with a published API I am committed to support. In an ongoing project I would probably have fixed that.

For issues I did not want to “fix” (because I considered that the issues were not real problems, and the “fix” was too convoluted or impossible), FindBugs provided me with the @SuppressWarnings annotation -which you should use scarcely!

All in all, making FindBugs happy was not difficult, enhanced my code in some cases, helped me uncover and fix one bug, and only in a few cases got in the way. And all of this setting it to the maximum strictness. A really good balance sheet!

Feasibility of a “cleanup campaign” for existing projects

Of course, you will need to make especial provisions if you want to introduce FindBugs in an ongoing project, unless you are prepared to launch a cleanup campaign that ensures your application has zero issues and uses FindBugs annotations (more on that later!).

Now, I have to say that the full cleanup campaign for DirectJNgine, with all settings set to the maximum strictness (!), took arount 10 to 12 hours, which I think is very little time.

If you take into account that I had to learn FindBugs way too, I’m quite sure that would have taken 6/8 hours if I had to do it now in a similar project.

Use FindBugs annotations -really!

Most people seem to be using FindBugs to check their code, but they seem not to be using FindBugs Annotations to help it diagnose even more problems.

Too bad, for these annotations are hidden gems. Use them! They will help FindBugs to check for adequate resource cleanup, correct null handling, etc. I have discussed them in other articles and posts about FindBugs.

My experience “porting” DirectJNgine to use these annoations was straightforward: 70 minutes only!

To be fair, I’m sure that was partly due to the fact that I have near 500 assertions in DJN. Many of these are checks for nulls,and that helped a lot when trying to make sure whether values were nullable or not.

While I do not expect most programs to be “ported” so easily, I don’t think it will take too much time.

That said, and even though I’m almost paranoid with checks for null values, FindBugs found two places where I had forgotten to assert that a value could not be null. So, again, a good result.

The Eclipse plugin

The Eclipse plugin for FindBugs works quite well. However, you need to know that some kinds of checks are not performed automatically when you save the source code.

Therefore, for some kinds of issues, you will need to manually run the FindBugs checker or rebuild the whole app. This happens only for some kinds of issues, and in most cases having real-time checking works and is useful, so not much of a problem if you are aware of this.

One caveat, though: DirectJNgine is a small project, so maybe FindBugs will be more taxing for medium to big projects. I’ll report on this when I introduce FindBugs in bigger projects.

On closing

FindBugs is an excellent and unobtrusive tool, worth using. In fact, I just hope the compiler could provide many of its checks as warnings.