More ways to try to maximize quality

Yesterday we looked at unit testing, especially test-driven development.  I'm going to add one more type of test here: a test for a private method (or any method which is otherwise out of scope).

Take a look at these two methods in the test class:

 @Test
 public void getTotal_A$() throws Exception {
  final Randomizer target = new Randomizer(getRandom());
  assertEquals(0, getTotal(target));
  target.setBands(1, 2, 3, 4);
  assertEquals(10, getTotal(target));
 }

 private int getTotal(final Randomizer target) throws Exception {
  Object total = TestUtilities.invokeInstanceMethod(target, "getTotal", new Class[] {}, new Object[] {}); //$NON-NLS-1$
  assertTrue(total instanceof Integer);
  return ((Integer) total).intValue();
 }

Most of the time, we can probably skip testing private methods.  But there are times when the private method contains some especially tricky logic, which is then further complicated by its caller(s) and we want to isolate the test to focus on precisely the private method.  I use a utility method which is in a class called TestUtilities which I put in the test directory along with the unit test classes.  The particular method used here is simple enough - I'll just show it in-lined (although the class actually has a number of methods with convenient signatures and is rather more elegant).

 public static Object invokeInstanceMethod(final Object instance, final String methodName, final Class[] paramClasses,
   final Object[] paramObjects) throws Exception {
  final Method method = instance.getClass().getDeclaredMethod(methodName, paramClasses);
  method.setAccessible(true);
  return method.invoke(instance, paramObjects);

Now let's take a look at some tools to try to improve code quality.  In a previous blog, I extolled the virtues of cleaning up code (using the cleanup tool in Eclipse, for example) before commiting to source control. However, perhaps I didn't sufficiently emphasize that it is not just formatting that is significant, but code cleanup. So, let's assume that our code has been cleaned up by Eclipse (or whatever) and compiles and passes all of its unit tests.

First, I want to take a look at the dreaded NullPointerException.  You definitely want to turn on the Java compiler warning which warns that this is possible ("Null pointer access" under "Potential programming problems"). This can find some possible problems, but it's not nearly as good in Eclipse Indigo as it is in Juno (Eclipse version 4).  Juno has annotations to allow you to explicitly set the expected behavior of references.  There are also the
javax.validation.constraints annotations like @NotNull though I'm not sure how useful these really in practice.  Until we can adopt the Juno annotations, we can try to avoid using variables (and initializing them to null) when we should be using constants.  And, unless it is explicitly covered as a potential and valid return value, methods should never return null.  If null is an invalid response it is better to throw an exception.  Assertions can also be used (by specying the -ea compiler option), but these have always seemed a crude way of doing things.  Annotations are much more appropriate.

I like another Eclipse plugin called FindBugs.  This will run on selected code (whole projects if you like) and come up with some potential bugs.  In this case, it takes mild exception to the test class, generating 7 warnings like the following:

RandomizerTest.java:54 Random object created and used only once in RandomizerTest.nextBoolean_A$()

This is not particularly helpful because we knew that already - this is the test class, remember?  Strangely, it doesn't seem to care that there are about a dozen potential null-pointer exceptions.  Occasionally, it will complain about something that you want to keep as is.  There is generally an annotation that you can specify which will tell it not to "bug" you about that in the future (similar to the annotations that you can use to switch off specific compiler warnings).
Then finally, when you've fixed all of the warnings that the compiler gives, and all of the bugs that FindBugs finds, and dealt with any null pointer issues, it's time to look for code "smells".  This is my favorite part.  Most of the time I just look and get that feeling that something's, well, fishy.  But you can do it more scientifically.  The tool I'm most familiar with, another Eclipse plugin, is called JDeodorant.

It has a strange user interface that I really dislike but it does work and can detect four "flavors" of code smell: God class, Long method, Type checking and Feature envy. Even though my example is quite short and reasonably well programmed, JDeodorant does find a few odors and with recommendations for fixing, for example under God class, it does suggest a refactoring and similarly under Long method.  It will offer to refactor the code for you.  This is all well and good if the changes are just to one file because you can revert to a previous version if you like.  But be careful if several files will be refactored, as feature envy often suggests.  I would only use that after checking everything else into source control.

Going beyond the level of source code, of course there are profiles and other performance enhancing tools but these are beyond the scope of this article.  There is one other tool that I like to help maintain architectural integrity of packages (packages are perhaps the least well thought-out aspect of Java).  It is called SonarJ and can really help with getting your house in order, architecturally speaking, by detecting cycles (A calls B, B calls C, C calls A, etc.).  It will recommend how to break these cycles by introducing interfaces.  Even a relatively simple package (or set of packages) can be surprisingly complex - and you really need a good tool to keep things in order.
OK, back to work!