ONJava.com    
 Published on ONJava.com (http://www.onjava.com/)
 See this if you're having trouble printing code examples


Towards Bug-Free Code

by Ashwin Jayaprakash
12/22/2004

"Bug-free code." Well, that's a bold statement to make about one's code. In August 2004, Mozilla announced that it would offer $500 for every serious bug found by security researchers. I wouldn't dare to make such a claim about my code on a regular basis, or I'd be broke in a month. However, if we make good use of some of the basic idioms and rules of thumb of design and programming, we can take a step closer towards software with fewer bugs. Any programmer worth his or her salt will agree that lately, design patterns have been overused to the point that the programmers start off directly with advanced patterns, while being completely ignorant of basic rules.

If you think that loose coupling is something to do with car parts, or if you find yourself amongst those who think test-driven development is only for developers who have a lot of time on their hands, then you should read further. In this article, you will learn how to design and develop your projects using some of the shiny new features of Java 1.5 that will result in fewer bugs and facilitate easy testing.

One of the fundamental constructs of an object-oriented programming language is an interface. As you might already know, an interface represents a set of contracts or basic behavior that allows the developer to switch the implementation without having to make changes to the code again and again.

A conscientious programmer will have a handful of interfaces in his program to ensure that the program is not too tightly coupled. An overzealous or novice programmer will be indiscriminate in creating too many interfaces and will still have packages that are neither stable nor easily extensible.

A line of code such as MyInterface intf = new MyInterfaceImpl() has little meaning, as the programmer has already decided that the implementation of MyInterface is MyInterfaceImpl right away. To a mature programmer, the solution seems obvious: use a factory pattern.

Hooking Implementations up to Interfaces

Let's assume that there are three interfaces with three respective implementations. The factory can be written as follows:


public class MyFactory
{
    Interface1 static getIntf1()
    {
    }

    Interface2 static getIntf2()
    {
    }

    Interface3 static getIntf3()
    {
    }
}
        

In a reasonably complex program, there could be around 10 to 15 interfaces. Imagine how unsightly the factory would look like with methods like getIntf1() to getIntf15(). For every new interface that gets introduced to the program, we'd have to add a new factory method, which does not sound like a good idea.

On the other hand, one could design a generic Factory that almost resembles a Map:


public class MyOtherFactory
{
    Object static getImplementationFor(Class
              interfaceClass)
    {
    }
}
        

The problem with this is that the key that uniquely identifies the implementation does not guarantee that the return type will be of the same type as the one expected by the caller. An explicit cast is required, like this:


MyInterface7 intf7 = (MyInterface7)
              MyOtherFactory.getImplementationFor(
              MyInterface7.class);
        

It is also prone to copy-and-paste mistakes like the one below, which cannot be identified until the program is run:


MyInterface7 intf7 = (MyInterface7)
              MyOtherFactory.getImplementationFor(
              MyInterface1.class);
        

With the introduction of generics in Java 1.5, we can design a factory/registry that is both type-safe and future-proof. Although the SimpleRegistry shown below is indeed very simple and has a few subtle shortcomings, the getImplFor(...) is the method on which to focus your attention. We'll talk about writing a better registry later.


public class SimpleRegistry
{
    private static final Map IMPL_MAP =
              new HashMap();

    static
    {
        IMPL_MAP.put(Number.class,
              new Integer(100));
    }

    public static <V> V getImplFor(
              Class<V> clazz)
    {
        final Object obj = IMPL_MAP.get(clazz);

        return clazz.cast(obj);
    }

    public static void main(String[] args)
    {
        Number number = SimpleRegistry.getImplFor(
              Number.class);

        System.out.println(number);
    }
}
        

The code that uses this is Number number = SimpleRegistry.getImplFor(Number.class). You will notice that there are no casts required, unlike the older MyOtherFactory example. The getImplFor(...) method says that it can accept a Class as a parameter, and also returns an Object of the same Class as the parameter. It would be impossible to do this:

Number number = SimpleRegistry.getImplFor(String.class) 

as the compile step would fail. You will also notice that it works with abstract classes (java.lang.Number) and even concrete classes like Integer integer = SimpleRegistry.getImplFor(Integer.class), although this does not make much sense.

If you want to streamline your interfaces, an empty/marker interface can be introduced that indicates that the implementation must be obtained from the registry. Drawing inspiration from electronic components, which are easily interchangeable, we'll name it Component:


public interface Component
{
}
        

Generic, Type-Safe Registry/Factory

The new and more practical registry to store all components will look like this:


public class Registry
{
    private final Map<Class<? extends
              Component>,
              Component> componentImplMap;

    protected Registry()
    {
        componentImplMap = Collections.
                synchronizedMap(
                new HashMap<Class<? extends
                Component>, Component>());
    }

    protected Component getComponentImpl(
                Class<? extends
                Component> componentClass)
    {
        /*
         Return the Component implemenation by
         looking up an internal map.
         Implementations can be cached too.
        */

        return componentImplMap.get(
                             componentClass);
    }

    public <T extends Component>
                void setComponentImpl(
                Class<T> componentClass,
                T componentImpl)
    {
        componentImplMap.put(componentClass,
                componentImpl);
    }

    //--- static fields and methods below ---

    protected static volatile Registry
                registry = null;

    public static void init()
    {
        getInstance();
    }

    public static Registry getInstance()
    {
        if (registry == null)
        {
            synchronized (Registry.class)
            {
                if (registry == null)
                {
                    registry = new Registry();
                }
            }
        }

        return registry;
    }

    /**
     * @param clazz
     * @return Instance of type clazz
     */
    public static <V extends Component> V
                getImplFor(Class<V> clazz)
    {
        final Registry registry = getInstance();

        final Component component =
                registry.getComponentImpl(clazz);

        return clazz.cast(component);
    }
}
        

This registry is used to store implementations of interfaces or abstract classes that ultimately implement the component interface. For example:


public interface MessagePublisher extends
                Component
{
    public void publish(Message msg)
                throws PublishException;
}

MessagePublisher publisher = Registry.getImplFor(
                MessagePublisher.class);
        

The registry will have to be populated with implementations as soon as the program starts up in the main(...) method or the startup hooks/classes that are allowed by some application servers.

Another advantage of this registry is that it truly is type-safe. The following are disallowed by the compiler itself:


//Cannot add non-Component instances.
Registry.getInstance().setComponentImpl(
                Integer.class, new Integer(100));

Registry.getInstance().setComponentImpl(
               Component.class, new Integer(100));

//Cannot add spurious mappings.
MessagePublisher anonPublisher =
                new MessagePublisher()
{
};

Registry.getInstance().setComponentImpl(
                MessageReceiver.class,
                anonPublisher);
        

Asking developers to write and run tests for the classes/modules they are working on before checking in their code is always met with resistance. "My module depends on another module and that is not yet ready," or "the code is still a work in progress," or "the system is too complicated to run a unit-level test." They continue, "Nothing short of a full-blown, end-to-end integration test will do. And that can be done only when everything is ready." Well, these were some of the reasons I myself must have given in the past, I must confess. But when you sit down to think about it, you'll see that the reasons are lame. Most of the time, the real reason is that the program design did not allow for easy unit testing. This is where essential object-oriented concepts such as the dependency inversion principle, the acyclic dependencies principle, the stable dependencies principle, and the rest prove to be extremely useful.

The reason why I'm harping on using interfaces or abstractions will become clearer next, where we take on the "boring" task of unit testing the program.

ProjectX: An Example

We will consider a fictitious project called "ProjectX." Let's assume that there are just two very simple use cases:

  1. Add a purchase order to a database.
  2. Retrieve new purchase orders from the database and publish them to a messaging system for processing.

Obviously, there are two main modules here: the messaging subsystem and the database access subsystem.

Let's say that the developer who is supposed to develop the messaging module falls sick and goes on leave for two weeks, leaving the module half-done. The only things that the developer implementing the second use case has are the database access module and the interfaces to both subsystems that were designed when the team worked together in the first few weeks of the project.

The crucial part of this implementation cycle is to continue with development and testing without the messaging module. This can be accomplished by providing a dummy implementation of the messaging interfaces. And the easiest way is to replace the proposed implementation with the dummy.


public class UseCase2
{
    public void retrieveAndPublishPOs()
    {
        MessagePublisher messagePublisher =
                new DummyMessagePublisher();

        ....

        /*
         Retrieve POs from Database
         and publish each one.
        */

        messagePublisher.publish(po);

        ....
    }
}
        

When the messaging module is ready, DummyMessagePublisher can be replaced by the actual implementation. However, you'll notice that even though we have taken the trouble of extracting an interface for the MessagePublisher, we've still not been able to break free from its implementation because the new DummyMessagePublisher() will have to be replaced each time by whatever implementation we come up with. Which brings us back to square one. This is where providing "hooks" into the system allows for a lot of flexibility. Consider how easy the whole thing would have been, had we used:


MessagePublisher messagePublisher =
                Registry.getImplFor(
                MessagePublisher.class)

//And set the "impl" using this
registry.setComponentImpl(MessagePublisher.class,
                new DummyPublisher())

//or this in the setUp() method.
registry.setComponentImpl(MessagePublisher.class,
                new MyJMSPublisher())
        

Unit-testing ProjectX

Now that we have made accommodations for introducing any implementation for the interfaces, we'll see how easy it is to unit test this use case even without the messaging subsystem.


import common.registry.Registry;
import junit.framework.TestCase;
import org.easymock.MockControl;
import projectx.data.POMessage;
import projectx.database.POTable;
...
import projectx.messaging.MessagePublisher;
...
import projectx.usecases.usecase2.
                PublishPurchaseOrder;

public class PublishTestCase extends TestCase
{
    @Override protected void setUp()
                throws Exception
    {
        final Registry registry =
                Registry.getInstance();

        //--- setup OraclePOTable ---

        final OraclePOTable oraclePOTable =
                new OraclePOTable();

        registry.setComponentImpl(POTable.class,
                oraclePOTable);

        //--- setup Mock MessagePublisher ---

        final MockControl publisherControl =
                MockControl.createControl(
                MessagePublisher.class);

        final MessagePublisher publisher =
                (MessagePublisher)
                publisherControl.getMock();

        //Record the methods that will be invoked.
        try
        {
            publisher.publish(new POMessage());
        }
        catch (PublishException e)
        {
            //This can never happen because we are
            //just recording the calls.
        }

        //Expect to be called getNumOfNewPOs()
        //number of times.
        publisherControl.setVoidCallable(
                oraclePOTable.getNumOfNewPOs());

        //Set the MockControl to replay the
        //recorded methods.
        publisherControl.replay();

        registry.setComponentImpl(
                MessagePublisher.class,
                publisher);
    }

 public void testSuccesslPublishWithoutPublisher()
                throws SQLException,
                InsufficientCredentialsException
    {
        final POTable poTable =
                Registry.getImplFor(POTable.class);

        final int numOfPOsForDemo =
                poTable.getNumOfNewPOs();

        //---------------------------------------

        PublishPurchaseOrder publishPurchaseOrder
                = new PublishPurchaseOrder();

        final int published =
                publishPurchaseOrder.publishNewPOs();

        //---------------------------------------

        assertEquals(numOfPOsForDemo, published);
    }
}
        

If you read the setUp() method of this JUnit test case, you will see that it requires just four lines of code using the EasyMock library to place a mock implementation of MessagePublisher and set it up to expect the publish() invocation a certain number of times. The testSuccessfulPublishWithoutPublisher() test makes note of the number of new POs that have to be published using poTable.getNumOfNewPOs() before running the test. Then, it runs the actual test of invoking publishPurchaseOrder.publishNewPOs(), which returns the number of POs that were retrieved from the database and successfully published to the MessagePublisher.

The PublishPurchaseOrder's publishNewPOs() method looks like this:


public int publishNewPOs() throws SQLException,
                InsufficientCredentialsException
{
    final POTable poTable =
                Registry.getImplFor(POTable.class);

    final MessagePublisher publisher =
                Registry.getImplFor(
                MessagePublisher.class);

    int success = 0;
    final CachedRowSet rowSet =
                poTable.getNewPOs();

    while (rowSet.next())
    {
        POMessage tempMessage = new POMessage();
        /*
         Populate the PO object with the data in
         the current row.
        */
        try
        {
            publisher.publish(tempMessage);
            rowSet.deleteRow();

            success++;
        }
        catch (PublishException e)
        {
            logger.error("Error occurred while " +
                "publishing PO: " +
                tempMessage.toString(), e);

            try
            {
                rowSet.undoDelete();
            }
            catch (SQLException e1)
            {
                logger.error("Undo operation on" +
                deleted PO failed: " +
                tempMessage.toString(),
                e1);
            }
        }
    }

    rowSet.acceptChanges();
    rowSet.close();

    return success;
}
        

Again, publishNewPOs() must rely only on interfaces (MessagePublisher and POTable) to remain useful.


public interface POTable extends Component
{
    /**
     * @param message
     * @return The Order number of this newly
     * created PO.
     * @throws SQLException
     */
    public long addPO(POMessage message)
                throws SQLException,
                InsufficientCredentialsException;

    public int getNumOfNewPOs()
                throws SQLException;

    public CachedRowSet getNewPOs()
                throws SQLException,
                InsufficientCredentialsException;

    public void deletePO(POMessage message)
                throws SQLException,
                InsufficientCredentialsException;
}

public interface MessagePublisher extends Component
{
    public void publish(Message msg)
                throws PublishException;
}
        

After having written this test case, which covers only the "success path," we soon run out of excuses for not writing tests. So the next step would be to write a test case and see how the PublishPurchaseOrder.publishNewPOs() fares when the MessagePublisher throws an Exception in the publish(...) method.

This time, in addition to using a mock MessagePublisher, we will also use a DummyPOTable that uses a mock CachedRowSet to record and finally, verify that error-handling/compensating methods get invoked when a PO does not get published successfully.

Negative Tests on ProjectX

We set the mock MessagePublisher to throw an Exception after successfully publishing two Messages and then succeed from the fourth Message onwards. So the third row must be retained on the database if the PublishPurchaseOrder.publishNewPOs() is written to handle exceptions correctly.


public class PublishTestCase2 extends TestCase
{
      public void testPublishErrorWithoutPublisherWithMockTable()
                throws SQLException,
                InsufficientCredentialsException
    {
        final Registry registry =
                Registry.getInstance();

        final int publishErrorAt = 3;
        final int undoDeleteforRows = 1;

        //Here we use a DummyPOTable to simulate
        //a Database. But we would've used a mock
        //CachedRowSet even if there were a
        //Database since the idea here is to check
        //if the POMessage which could not be
        //published will still be retained in the
        //Database by invoking undoDelete() on
        //that row.

        final POTable poTable =
                getMockTableDemoForErrorTest(
                undoDeleteforRows);

        registry.setComponentImpl(POTable.class,
                poTable);

        /*
           Setup Mock MessagePublisher to throw
           Exception at publishErrorAt,
           then resume successfully.
        */

        final MockControl publisherControl =
                MockControl.createControl(
                MessagePublisher.class);

        final MessagePublisher publisher =
                (MessagePublisher)
                publisherControl.getMock();

        try
        {
            publisher.publish(new POMessage());
        }
        catch (PublishException e)
        {
            //This can never happen because we are
            //just recording the calls.
        }

        //Expect to be called "publishErrorAt - 1"
        //number of times before Mock is made to
        //throw an exception.
        publisherControl.setVoidCallable(
                publishErrorAt - 1);

        try
        {
            publisher.publish(new POMessage());
        }
        catch (PublishException e)
        {
            //This can never happen because we are
            //just recording the calls.
        }
        publisherControl.setVoidCallable(1);
        publisherControl.setThrowable(
                new PublishException(
                "Message from Mock - " +
                "Publish error occurred!!"));

        try
        {
            publisher.publish(new POMessage());
        }
        catch (PublishException e)
        {
            //This can never happen because we are
            //just recording the calls.
        }
        //Publish will be called on the remaining
        //Messages.
        publisherControl.setVoidCallable(
        poTable.getNumOfNewPOs() - publishErrorAt);

        publisherControl.replay();

        registry.setComponentImpl(
               MessagePublisher.class, publisher);

        //---------------------------------------

        final int numOfPOsForDemo =
                poTable.getNumOfNewPOs();

        //---------------------------------------

        PublishPurchaseOrder publishPurchaseOrder =
                new PublishPurchaseOrder();

        final int published =
                publishPurchaseOrder.publishNewPOs();

        //---------------------------------------

        assertEquals(
              numOfPOsForDemo - undoDeleteforRows,
              published);
    }
        

The code below uses EasyMock to generate a "canned" CachedRowSet that is configured to have five new POs. It also sets it to expect an undo operation on undoDeleteforRows number of those POs. Then it sets the DummyPOTable to use this CachedRowSet for unit testing.


    private POTable getMockTableDemoForErrorTest(
                final int undoDeleteforRows)
                throws SQLException
    {
        final MockControl rowSetControl =
                MockControl.createNiceControl(
                CachedRowSet.class);

        final int newRowsForDemo = 5;

        final CachedRowSet rowSetForDemo =
                (CachedRowSet)
                rowSetControl.getMock();

        rowSetForDemo.next();
        rowSetControl.setReturnValue(true,
                newRowsForDemo);

        rowSetControl.setReturnValue(false,
                MockControl.ZERO_OR_MORE);

        rowSetForDemo.deleteRow();
        rowSetControl.setVoidCallable(
                newRowsForDemo);

        rowSetForDemo.undoDelete();
        rowSetControl.setVoidCallable(
                undoDeleteforRows);

        rowSetForDemo.acceptChanges();

        rowSetForDemo.close();

        rowSetControl.replay();

        //Since this is a demo, we'll simulate a
        //live Database by just returning these
        //data.

        return new DummyPOTable(newRowsForDemo,
                rowSetForDemo);
    }
}
        

The getMockTableDemoForErrorTest(...) method unveils the true power of EasyMock, by allowing us to create a mock/dummy CachedRowSet using just 11 lines of code. Otherwise, we would have been required to implement a total of around 248 methods in the CachedRowSet interface!

Now that we've designed our program carefully, you will also find its modular design invaluable when it comes to replicating or reproducing errors. Error conditions or production configurations can be easily setup to study and fix bugs quickly. Figures 1 and 2 show that the design is indeed modular, with implementations clearly separated from and dependent on abstract, stable classes in separate packages

Figure 1.
Figure 1. Dependencies inside of ProjectX are acyclic

Figure 2.
Figure 2. ProjectX's dependence on the common package is acyclic, and common is stable as it is made up mostly of abstract classes and interfaces

Since we are on the topic of writing bug-free code, there's another place where Java 1.5 features can prove to be very helpful. If you've worked on projects that required internationalization (I18N), then you must be familiar with ResourceBundles.

The most common way of creating locale-specific bundles is by using property files. But the problem with having message keys and values in the property files is that the code that refers to those messages uses plain strings. It is very possible to have spelling mistakes in either the code using the keys or in one of the several locale-specific bundles. Even if a key has been renamed in the bundle, the code can still continue to refer to the old key, since it is just a string and will compile successfully. There's no way of knowing if there's such a mistake until the whole project's code is executed and verified. That could be tiresome or just too late in the process.

I18N using Type-Safe Resource Bundles

A way of preventing such mistakes from happening is by using a ListResourceBundle where the locale-specific messages are written as Java classes instead of as property files. The ListResourceBundle can be enhanced to accept keys belonging to an enum type. If the keys are wrong, then the code will fail the compilation step and it can be corrected immediately. The task becomes easier if you use a good Java editor that has an auto-completion feature.


public abstract class
                AbstractTypeSafeResourceBundle
                <K extends Enum<K>>
                extends ListResourceBundle
{
    private final EnumMap<K, Object> catalog;
    private final Object[][] cachedCatalog;

    public AbstractTypeSafeResourceBundle(
                Class<K> clazz)
    {
        catalog = new EnumMap<K, Object>(clazz);

        loadCatalogMap();

        //---------------------------------------

        cachedCatalog =
                new Object[catalog.size()][2];

        int i = 0;
        for (K key : catalog.keySet())
        {
            cachedCatalog[i][0] = key.name();
            cachedCatalog[i][1] = catalog.get(key);
            i++;
        }
    }

    public EnumMap<K, Object> getCatalogCopy()
    {
        return catalog.clone();
    }

    protected final void load(K key, Object obj)
    {
        catalog.put(key, obj);
    }

    protected abstract void loadCatalogMap();

    //--- ListResourceBundle methods ---

    protected final Object[][] getContents()
    {
        return cachedCatalog;
    }
}
        

Each module or project should create an enum, which will serve as the set of keys to the bundle:


public enum ResourceKey
{
    info_Hello,
    info_GoodMorning;
}
        

The locale-specific bundles will look like this:


//English version.
public class ProjectXResBundle extends
AbstractTypeSafeResourceBundle<ResourceKey>
{
    public ProjectXResBundle()
    {
        super(ResourceKey.class);
    }

    protected void loadCatalogMap()
    {
        load(ResourceKey.info_GoodMorning,
                "Good morning");

        load(ResourceKey.info_Hello, "Hello!");
    }
}

//German version.
public class ProjectXResBundle_de_DE extends
AbstractTypeSafeResourceBundle<ResourceKey>
{
    public ProjectXResBundle_de_DE()
    {
        super(ResourceKey.class);
    }

    protected void loadCatalogMap()
    {
        load(ResourceKey.info_GoodMorning,
                "Guten Morgen");

        load(ResourceKey.info_Hello, "Hallo!");
    }
}
        

A direct way of using the bundles would be as shown below:


final ResourceBundle englishResourceBundle =
                ResourceBundle.getBundle(
                ProjectXResBundle.class.getName());

System.out.println(
                englishResourceBundle.getString(
                ResourceKey.info_Hello.name()));


final ResourceBundle germanResourceBundle =
                ResourceBundle.getBundle(
                ProjectXResBundle.class.getName(),
                new Locale("de", "DE"));

System.out.println(germanResourceBundle.getString(
                ResourceKey.info_Hello.name()));
        

Or you can define a helper class that takes care of substituting the parameters to construct the localized message by using the varargs feature in 1.5:


public class Helper
{
    public static String format(ResourceBundle
                resourceBundle,
                Enum key,
                Object... paramsForI18NMsg)
    {
        final Object l10nObj =
                resourceBundle.getObject(key.name());

        final String messagePattern =
                l10nObj.toString();

        if (paramsForI18NMsg.length > 0)
        {
            final MessageFormat formatter =
                new MessageFormat(messagePattern,
                resourceBundle.getLocale());

            return formatter.format(paramsForI18NMsg);
        }

        return messagePattern;
    }
}
        

If the message takes time as a parameter to format according to locale, it will look like this (standard ResourceBundle conventions):


public class ProjectXResBundle_de_DE extends
AbstractTypeSafeResourceBundle<ResourceKey>
{
    public ProjectXResBundle_de_DE()
    {
        super(ResourceKey.class);
    }

    protected void loadCatalogMap()
    {
        load(ResourceKey.info_GoodMorning,
                "Guten Morgen {0,time,short} - {1}");

        load(ResourceKey.info_Hello, "Hallo!");
    }
}
        

With the substitution parameters forming the varargs, the code looks a lot simpler and cleaner:


//No parameters, yet no unsightly "new Object[]{}"
//that had to be used until 1.5.
System.out.println(Helper.format(
                germanResourceBundle,
                ResourceKey.info_Hello));

System.out.println(Helper.format(
                germanResourceBundle,
                ResourceKey.info_GoodMorning,
                new Date(), "Ashwin"));
        

The last message prints "Guten Morgen 17:58 - Ashwin" with the time in the 24-hour format as it is used in Germany.

The source code contains a package called common, which takes the Registry a step further by allowing ComponentFactories to be added to the Registry instead of just Component implementations (if required) and a generic logging framework that allows enums to be used for internationalized messages in conjunction with AbstractTypeSafeResourceBundle. The projectx package has examples that use these.

Resources

Ashwin Jayaprakash is a software engineer at BEA Systems R & D Centre, Bangalore, using Java/J2EE to develop WebLogic Integration products.


Return to ONJava.com.

Copyright © 2009 O'Reilly Media, Inc.