Java Tip 88: Complement testing with code inspections

Avoid common mistakes by conducting regular code inspections

Page 2 of 2

You might not catch this mistake during testing. First, testing for threading bugs is very difficult and time consuming. Second, on some CPUs this code might translate into a single instruction, and thus work correctly. The bug would appear only when tested on other JVMs. It is better to simply synchronize the code correctly from the start:

    public synchronized void increment ()
    {
        ++this.value;
    }

Common mistake #9: Putting cleanup code in a catch block

Putting cleanup code in a catch block often results in code that looks like this:

    OutputStream os = null;
    try
    {
        os = new OutputStream ();
        // Do something with os here.
        os.close();
    }
    catch (Exception e)
    {
        if (os != null)
            os.close();
    }

Although this code is wrong for several reasons, the mistake(s) could easily be overlooked during testing. Here are three problems with this code:

  • The os.close() statement appears in two places. This is unnecessary as well as a potential maintenance problem.
  • The code above handles only Exceptions, not Errors. Even if the try block fails with an Error, the stream should be closed.
  • close() can throw an IOException.

A much-improved version is:

    OutputStream os = null;
    try
    {
        os = new OutputStream ();
        // Do something with os here.
    }
    finally
    {
        if (os != null)
            os.close();
    }

This repairs the first two problems: code is no longer duplicated and Errors are handled properly. There are no good solutions to the last problem. Your best solution is probably to put the close() inside a try/catch block itself.

Common mistake #10: Adding unnecessary catch blocks

Some developer hear the term try/catch block and assume that all try blocks must have an associated catch block. This is reinforced for C++ programmers because C++ has no concept of a finally block. In C++, the only reason to have a try block is to pair it with an associated catch block.

The result of adding unnecessary catch blocks is code like this, where exceptions are caught and then immediately rethrown:

    try
    {
        // Nifty code here
    }
    catch (Exception e)
    {
        throw e;
    }
    finally
    {
        // Cleanup code here
    }

You can shorten the above code by eliminating the unnecessary catch block:

    try
    {
        // Nifty code here
    }
    finally
    {
        // Cleanup code here
    }

Common mistake #11: Failing to implement equals, hashCode, or clone

The default implementations of equals, hashCode, and clone, provided by java.lang.Object, are correct. Unfortunately, these implementations are not always useful. To correct this, many classes override one or more of these methods to provide more useful functionality.

Unfortunately, when extending a class that overrides one or more of these methods, the child class usually needs to override the methods, too. Code reviews should check to ensure that if parent classes implement equals, hashCode, or clone, the child classes are still correct. Writing correct implementations of equals, hashCode, and clone is tricky. The Resources section provides a link to an article explaining how you can do this.

Conclusion

I have seen each of these mistakes in at least one code inspection. In fact, I've made more than one of them myself. The good news is that code inspections are easy to administer, and the mistakes are easy to find and fix, as long as you know what to look for. Even if you can't find the time for formal code inspections, rooting out these mistakes from your own code will save you debugging time. The time spent reviewing will be time well spent.

Mark is the Java Tip technical coordinator for JavaWorld. He has been programming professionally since 1989 and has been using Java since the alpha-3 release. He works full time at KLA-Tencor and is part of a team building a 500KLOC distributed parallel multicomputer application for image processing (among other things) that is written almost entirely in Java.

Learn more about this topic

| 1 2 Page 2