2 January 2008 - 11:08Don’t call subclass methods from a superclass constructor

When designing a class for subclassing, it’s important to avoid calling any method that the subclass could or must override from the superclass constructor. This includes any non-final public or protected methods (either abstract or concrete). The only methods that are appropriate to call from the superclass constructor are final methods or private methods in the superclass.

Unfortunately, this mistake is all too easy to make - I’ve made it myself lots of times. Usually, this is done because the superclass has some invariant that must be enforced, and part of enforcing the invariant involves calling at least one method that the subclass could or must override.

For example, consider the following contrived class:

class SuperClass {
    private final FooBar fooBar;
    private String message;

    public SuperClass(FooBar x) {
        fooBar = x;
        message = computeMessage(x);
    }

    protected String computeMessage(FooBar x) {
        return x.someMethod() + "..."; // details of the message are not important for this example
    }
}

This class exhibits the problem this entry describes. Class ‘SuperClass’ is designed for subclassing, but it calls an abstract method from its constructor. The intention of this class is to enforce the invariant that the ‘message’ field will never be null after construction, and computing the correct value for the ‘message’ field involves the subclass. Even though this is a contrived example, many needs for calling subclass methods from a superclass constructor have this form in essence.

The reason this is bad is because in Java, superclass constructors run before subclass constructors (that is, a subclass must invoke super() before other statements in its constructors). By invoking a subclass method, code in the subclass will be invoked before the subclass gets a chance to initialize itself. The subclass will see default values for all its fields, and will not have access to any data passed in the subclass constructor.

For example, consider the following subclass of the class given above:

class SubClass extends SuperClass {
    private MessageHelper helper;

    public SubClass(FooBar x, MessageHelper helper) {
        super(x);
        this.helper = helper;
    }

    protected String computeMessage(FooBar x) {
        return helper.getMessage(x); // NullPointerException!
    }
}

In this example, the subclass takes a helper object in its constructor. The helper object is used in the overridden abstract method. However, because the superclass constructor calls the abstract method, the subclass does not have a chance to assign the helper object to a field (its constructor has not run yet). This requires the subclass to either guard for this condition (but what should it do when the condition happens?) or contain a NPE bug as in the example above.

Fortunately, this problem is easy to fix with the following design pattern. First, observe that there are two issues at play here: (1) enforcing an invariant and (2) designing for subclassing. The best way to satisfy both issues is to make the superclass abstract and require (through documentation) that all subclasses enforce the invariant in their constructor.

For example, here is a fixed version of ‘SuperClass’ and ‘SubClass’ from above that fix the problem:

abstract class SuperClass {
    private final FooBar fooBar;
    private String message;

    protected A(FooBar x) {
        fooBar = x;
    }

    protected final void initialize() {
        message = computeMessage(fooBar);
    }

    protected abstract String computeMessage(FooBar x);
}

class SubClass extends SuperClass {
    private MessageHelper helper;

    public SubClass(FooBar x, MessageHelper helper) {
        super(x);
        this.helper = helper;
        initialize(); // subclass MUST call initialize
    }

    protected String computeMessage(FooBar x) {
        return helper.getMessage(x); // no NullPointerException
    }
}

The superclass is made abstract to avoid the possibility of creating an instance in which the invariant is not satisfied. This is OK, since a fundamental part of this design is to allow for subclassing. In addition, all subclasses must call the protected initialize() method from their subclass constructor. If they fail to do so, the superclass-subclass contract is broken and invariants can’t be guaranteed.

The fix may be considered a little cumbersome. It requires the creation of at least one other class (since before the fix, the superclass could be used directly). The fix also requires (but can’t enforce through the compiler) that subclasses all follow certain rules. However, these tradeoffs are much better that having the situation in which subclass code is invoked before the subclass can initialize itself. By the way, I am pretty sure this issue is also discussed in Effective Java (I’d be sure, but I don’t have a copy handy at the moment).

Comment summary:

One commenter asked: Does this relate to Bug 6596304? http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6596304

I replied:

Yep, that issue has the same root cause. It’s a particularly insidious example of how things can go wrong when this guideline is broken.

If you compile and disassemble the code provided in that bug, it’s pretty easy to see what’s going on. The Child class’s constructor contains bytecode to set all of Child’s fields to their default values (e.g. aconst_null, putfield). This code runs after the code that sets the Child’s field value in the overridden init() method. It undoes the set to the field that happened when the superclass constructor called the overridden method.

The moral of the story is that calling any method that can be overridden from a superclass constructor is a dodgy practice and should be avoided.

Thanks for the link to the bug - that makes a great example for explaining this issue.

Eric Wilson said: C# has the same rules for constructor execution, so it can also be subject to the same issue.