Login | Register   
LinkedIn
Google+
Twitter
RSS Feed
Download our iPhone app
TODAY'S HEADLINES  |   ARTICLE ARCHIVE  |   FORUMS  |   TIP BANK
Browse DevX
Sign up for e-mail newsletters from DevX


advertisement
 

Find the Java Bugs That Pose a Threat with FindBugs : Page 3

FindBugs enables you to isolate and correct dangerous Java bugs. Its unique features separate it from the many static analysis tools in the Java world. Find out what makes FindBugs so special.


advertisement
FindBugs in Practice: Beyond Checkstyle and PMD
FindBugs is just one of many available static analysis tools, both open source and commercial. Two other commonly used static analysis tools in the Java arena are Checkstyle and PMD. Checkstyle has traditionally focused on coding standards such as naming conventions and spacing, and the presence of Javadocs. PMD is more focused on best practices, sub-optimal code, and potential errors. Some overlap inevitably exists between static analysis tools, especially between Checkstyle and PMD and, to a lesser extent, between PMD and FindBugs.

To get a better idea of the areas in which FindBugs shines, consider a practical example. Suppose you have to implement a database of registered pet dogs for a government agency. The following class represents the main domain object, the pet dog:

public class Dog { private String name; private String breed; // Getters and setters //... }



Now suppose that at some point during the project, you need to implement the equals() method (for example, you need to place Dog instances in a set). However, correctly implementing the equals() method can be a tricky operation, especially for novice programmers. Here is a relatively poor implementation of the equals() method for the Dog class:

public class Dog { private String name; private String breed; @Override public boolean equals(Object obj) { Dog otherDog = (Dog) obj; if ((otherDog.name.equals(this.name)) && (otherDog.breed.equals(this.breed))) { return true; } return false; } }

This implementation contains several mistakes, coding errors that could lead the application to crash or behave in unexpected ways. In particular, if the specified object is not an instance of the Dog class, the method will through a ClassCastException. There is no test to make sure the object is not null. And, although the equals() method has been overridden, there is no matching overridden hashCode() implementation. The Java language requires equal objects to return equal hashcode values. If they don't, HashMaps and HashTables are likely to behave incorrectly. So, whenever you override the equals() method, you should also override the hashCode() method. This rule, well known to veteran Java developers, is often overlooked by less experienced developers.

The implementation also shows a few poor coding practices, such as the lack of a Javadoc comment.

Each of the static analysis tools mentioned above will raise different issues for this code. For example, Checkstyle will indicate the following errors:

  1. The method is missing a Javadoc comment.
  2. The parameter obj should be final (as it is not modified in the method).
  3. There is a definition of equals() without a corresponding definition of hashCode().

Of these errors, the first is a coding standards issue, the second is a coding best practice, and the last is a potential bug as well as a well-known best practice. This error report reflects the general emphasis that Checkstyle places on enforcing coding standards and good programming habits.

PMD is more focused on best practices. When you run PMD against this code, it will raise the following issues:

  1. "A method should have only one exit point, and that should be the last statement in the method."
  2. "Ensure that you override both equals() and hashCode()."
  3. "Parameter obj is not assigned and could be declared final."

Of these three issues, two were also raised by Checkstyle. The issue raised only by PMD in this case concerns a common Java coding best practice: limit the number of return statements in a method in order to reduce the complexity of the code. This exemplifies PMD's focus on best practices. PMD is not concerned by the lack of Javadoc comments, nor would it worry about method or variable naming conventions, spacing and indentation, or other pure coding standards issues.

Now take a look at what FindBugs comes up with. Like the other tools, FindBugs raises three issues with this code, but their nature is quite different from those raised by the other two tools:

  1. The equals method should not assume anything about the type of its argument.
  2. The class defines equals() and uses Object.hashCode().
  3. The equals() method does not check for null arguments.

The first error finds one of the major flaws in the design of this equals() method: if the argument is not an instance of the Dog class, the method will fail. FindBugs' detailed description of this issue can also help explain both the problem and the solution:

"The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this."

The second issue is the same equals()/hashCode() issue raised by the other two tools. However, the FindBugs description of this issue goes much further in explaining the reasons behind the problem and how to fix it:

"This class overrides equals(Object), but does not override hashCode(), and inherits the implementation of hashCode() from java.lang.Object (which returns the identity hash code, an arbitrary value assigned to the object by the VM). Therefore, the class is very likely to violate the invariant that equal objects must have equal hashcodes. If you don't think instances of this class will ever be inserted into a HashMap/HashTable, the recommended hashCode implementation to use is:

public int hashCode() { assert false : "hashCode not designed"; return 42; // any arbitrary constant will do }"

The third issue is another potential bug that only FindBugs detected: the method fails to check for a null value in the parameter. The issue raised by FindBugs is actually even more precise than the error message indicates, and the description both details the problem and suggests a simple solution:

"This implementation of equals(Object) violates the contract defined by java.lang.Object.equals() because it does not check for null being passed as the argument. All equals() methods should return false if passed a null value."

This simple example illustrates two major differences that distinguish FindBugs from other static analysis tools:

  • FindBugs concentrates almost exclusively on coding issues that may result in application bugs.
  • FindBugs usually couples each issue it finds with a clear and detailed description, which can be used both to help understand the issue and to fix the problem.

Consider another example:

public void foo() { List<String> values = null; List<Integer> results = new ArrayList<Integer>(); for (String str : values) { int value = Integer.parseInt(str); results.add(new Integer(value)); } }

In this case, Checkstyle will not raise any significant issues, other than the lack of a Javadoc comment. Unless you configure Checkstyle carefully, it will also complain about the lack of spaces around the generic '<' and '>' operators. As per usual, Checkstyle is concentrating on the coding style and layout.

Figure 4. FindBugs Analysis in Action: A null pointer deference for the 'values' variable results in a NullPointerException during execution.

The default configuration of PMD will raise six issues, mostly to do with performance optimizations, including:

  1. The variables values, results, and value could be final. (This is theoretically correct, but rarely done in practice.)
  2. Avoid instantiating variables within loops. (This is a legitimate concern.)
  3. Found 'DU'-anomaly for variable results. (This is somewhat obscure.)

FindBugs, on the other hand, raises only two issues:

  1. There is a null pointer deference for the values variable. This would result in a NullPointerException during execution (see Figure 4).
  2. The use of new Integer(value) is inefficient, and should be replaced by Integer.valueOf(value), which avoids the creation of unnecessary objects (A potential performance issue similar to the second issue raised by PMD).

Again, this illustrates FindBugs' tendency to focus on potential bugs.

All static analysis tools raise false alerts. However, in practice, a high proportion of the issues raised by FindBugs turn out to be real bugs—often more than half, in my experience.



Comment and Contribute

 

 

 

 

 


(Maximum characters: 1200). You have 1200 characters left.

 

 

Sitemap