Code Analyzer – PMD

If you have a code review process in place, how do you make sure that the review is fruitful. The last thing you want is to spend many hours across different reviews pointing out the same thing over and over again. There are common code mistakes that we all make and it is impossible for a developer to catch them all. This is were tools such as CheckStyle and PMD among others come into picture. These are static code analyzers that will run through the code and check for conformance to a slew of good programming conventions (rules).

For example lets say you are a diligent developer who never eats exceptions. But then one day, without realizing, it happens. How do you catch it? You need a code analyzer if you want to guarantee finding the bad code.

PMD defines a whole set of rules grouped into rule sets. You can choose to apply only those rule sets of interest to your project. Of course you can add your own rules if you choose. PMD integrates with Eclipse(among other IDE’s) and Ant (makes it perfect for any Continuous Integration environment). If you really care about code quality make sure to have this setup in your IDE and in Ant. One without the other is useless. Having it in your IDE forces you to write good code from the beginning. This may seem painful in the beginning but in a few days writing code that conforms to the rule will be second nature to you.Anything you miss will be caught in the reports generated by PMD (you can get HTML reports or get an XML and apply whatever style sheet you want over that).

I do not have a detailed comparison between PMD and CheckStyle. But the one difference i seem to recollect is that CheckStyle can also be configured to check for JavaDoc and code style conventions. By codestyle I mean things like does the braces appear on a new line vs the same line as the function, and so on. But if code quality is the main goal then PMD will suffice.

One interesting feature of PMD is copy-paste detection or CPD. Need I say more about that .

To give you a flavor of some of the rule sets, here is one related to Jakarta logging. In the case where the application is logging exceptions this rule set checks that we conform to the two parameter logging API.

try {

// some code

}

catch(SomeException ex) {

log.error(“some message”, ex);

// other exception handling code

}

If we did log.error(ex)we actually just ate the stack trace and that canbe a bad thing. But I have made that mistake in the past without realizing it and then had it pointed out to me or simply realized itwhenthe stack trace did not show up. But now we are depending on one of the two happening to find out the problem. God forbid there is aproduction problem, that cannot be replicated in other environments and you realized you just ate up the stack trace.

The second rule enforces that we have a ‘private static final Log‘ instance with the current class passed to LogFactory.getLog.

private static final Log logger =LogFactory.getLog(Account.class);

would be the right thing in the Account class. But if you changed it to

private static final Log logger =LogFactory.getLog(Customer.class)

then that is wrong. Well it will work but why would you want to log messages under Customer class when you are in Account.