Java Tip 88: Complement testing with code inspections

Avoid common mistakes by conducting regular code inspections

By Mark Roulo, JavaWorld.com, 02/07/00

Formal code inspections are one of the most powerful techniques available for improving code quality. Code inspections -- peers reviewing code for bugs -- complement testing because they tend to find different mistakes than testing does.

Code inspections are even more useful when inspectors hunt for specific errors rather than casually browse for bugs. In this article, I present 11 common mistakes that have been made at one time or another in the Java code base on which I am currently working. You can make these items into a checklist for your own code inspections. Then you can be confident that your code doesn't repeat these typical mistakes.

Common mistake #1: Copying Strings excessively

One mistake that you won't detect by testing is making unnecessary copies of immutable objects. Immutable objects cannot change, so there is no need to copy them. The most commonly used class of immutable objects is String. If you must change the contents of a String, you must instead use a StringBuffer.

So, copying String in this way would work:

    String s = new String ("Text here");

The code, however, is unnecessarily slow and complicated. You could also write the above code as:

    String temp = "Text here";

    String s = new String (temp);

But this rewrite also contains an extra, unnecessary String. A better way to write this is by simply writing:

    String s = "Text here";

One warning: this may not be true if you run a pre- or post-processor on your code, as, for example, many ODMG-93 databases do. The pre- or post-processor can modify your code so that what appear to be String objects really aren't.

Common mistake #2: Failing to clone returned objects

Encapsulation is one of the key tenets of object-oriented programming. Unfortunately, Java makes it easy to accidentally break encapsulation by returning references to private data.

The following class illustrates this:

    import java.awt.Dimension;
    /**

     * Example class.  The x and y values should never

     * be negative.

     */

    public class Example

    {

        private Dimension d = new Dimension (0, 0);
        public Example ()

        {

        }
        /**

         * Set height and width.  Both height and width must be nonnegative

         * or an exception is thrown.

         */

        public synchronized void setValues (int height, int width)

            throws IllegalArgumentException

        {

            if (height < 0 || width < 0)

                throw new IllegalArgumentException();
            d.height = height;

            d.width = width;

        }
        public synchronized Dimension getValues()

        {

            // Ooops!  Breaks encapsulation

            return d;

        }

    }

The Example class promises that the height and width values stored inside will never be negative. Attempting to set negative values with the setValues() method fails with an exception. Unfortunately, because getValues() returns a reference to d instead of a copy of d, you can write the following nasty code: