Saturday, January 29, 2005

Oops, null

I often rail against returning null as a marker or default value. I should listen to my own advice!

I'm working on a complex area of a program and deep within its structure is a method for finding the current main window frame. Originally, there might have been several top level frames so they were kept in a global list. But more recently the software moved to using one JVM for each main window, a good approach in light of the strong improvements in JDK 5 with sharing between JVMs.

Enforcing this rule, I wrote this:

public Frame getMainWindow() {
    final List<Frame> list = getTopLevelWindows();

    if (list.size() != 1)
        throw new IllegalStateException(
                "Other than one program on this VM");

    return list.get(0);
}

But this broke automated unit tests which exercise sections of the code calling getMainWindow. Tests do not have a main program window. Ok. So how did I fix it?

public Frame getMainWindow() {
    final List<Frame> list = getTopLevelWindows();

    if (list.size() > 1)
        throw new IllegalStateException(
                "More than one program on this VM");

    return list.isEmpty() ? null : list.get(0);
}

WRONG! I'm now returning null, but what if code is broken elsewhere and the list of top level windows is empty when it should have a member? I've created an NullPointerException at some random location instead of a specific failure. I've ignored my own advice.

What is the correct solution? Well, as the class containing this method implements an interface including getMainWindow and the unit test is already using a delegate instance of the interface, I restore the original version of the method, and implement a test-only version in the testing delegate:

@Override public Frame getMainWindow() {
    assertTrue(getTopLevelWindows().isEmpty());

    return null;
}

I now have correct behavior: running production code demands one and only one top level window per JVM, and unit test code demands none. I wish I had done this in the first place.

Post a Comment