[java] What's wrong with overridable method calls in constructors?

I have a Wicket page class that sets the page title depending on the result of an abstract method.

public abstract class BasicPage extends WebPage {

    public BasicPage() {
        add(new Label("title", getTitle()));
    }

    protected abstract String getTitle();

}

NetBeans warns me with the message "Overridable method call in constructor", but what should be wrong with it? The only alternative I can imagine is to pass the results of otherwise abstract methods to the super constructor in subclasses. But that could be hard to read with many parameters.

I guess for Wicket it's better to call add method in the onInitialize() (see components lifecycle) :

public abstract class BasicPage extends WebPage {

    public BasicPage() {
    }

    @Override
    public void onInitialize() {
        add(new Label("title", getTitle()));
    }

    protected abstract String getTitle();
}

Here is an example that reveals the logical problems that can occur when calling an overridable method in the super constructor.

class A {

    protected int minWeeklySalary;
    protected int maxWeeklySalary;

    protected static final int MIN = 1000;
    protected static final int MAX = 2000;

    public A() {
        setSalaryRange();
    }

    protected void setSalaryRange() {
        throw new RuntimeException("not implemented");
    }

    public void pr() {
        System.out.println("minWeeklySalary: " + minWeeklySalary);
        System.out.println("maxWeeklySalary: " + maxWeeklySalary);
    }
}

class B extends A {

    private int factor = 1;

    public B(int _factor) {
        this.factor = _factor;
    }

    @Override
    protected void setSalaryRange() {
        this.minWeeklySalary = MIN * this.factor;
        this.maxWeeklySalary = MAX * this.factor;
    }
}

public static void main(String[] args) {
    B b = new B(2);
    b.pr();
}

The result would actually be:

minWeeklySalary: 0

maxWeeklySalary: 0

This is because the constructor of class B first calls the constructor of class A, where the overridable method inside B gets executed. But inside the method we are using the instance variable factor which has not yet been initialized (because the constructor of A has not yet finished), thus factor is 0 and not 1 and definitely not 2 (the thing that the programmer might think it will be). Imagine how hard would be to track an error if the calculation logic was ten times more twisted.

I hope that would help someone.

If you call methods in your constructor that subclasses override, it means you are less likely to be referencing variables that don’t exist yet if you divide your initialization logically between the constructor and the method.

Have a look on this sample link http://www.javapractices.com/topic/TopicAction.do?Id=215

Invoking an overridable method in the constructor allows subclasses to subvert the code, so you can't guarantee that it works anymore. That's why you get a warning.

In your example, what happens if a subclass overrides getTitle() and returns null ?

To "fix" this, you can use a factory method instead of a constructor, it's a common pattern of objects instanciation.

In the specific case of Wicket: This is the very reason why I asked the Wicket devs to add support for an explicit two phase component initialization process in the framework's lifecycle of constructing a component i.e.

1. Construction - via constructor
2. Initialization - via onInitilize (after construction when virtual methods work!)

There was quite an active debate about whether it was necessary or not (it fully is necessary IMHO) as this link demonstrates http://apache-wicket.1842946.n4.nabble.com/VOTE-WICKET-3218-Component-onInitialize-is-broken-for-Pages-td3341090i20.html)

The good news is that the excellent devs at Wicket did end up introducing two phase initialization (to make the most aweseome Java UI framework even more awesome!) so with Wicket you can do all your post construction initialization in the onInitialize method that is called by the framework automatically if you override it - at this point in the lifecycle of your component its constructor has completed its work so virtual methods work as expected.

Here's an example which helps to understand this:

public class Main {
    static abstract class A {
        abstract void foo();
        A() {
            System.out.println("Constructing A");
            foo();
        }
    }

    static class C extends A {
        C() { 
            System.out.println("Constructing C");
        }
        void foo() { 
            System.out.println("Using C"); 
        }
    }

    public static void main(String[] args) {
        C c = new C(); 
    }
}

If you run this code, you get the following output:

Constructing A
Using C
Constructing C

You see? foo() makes use of C before C's constructor has been run. If foo() requires C to have a defined state (i.e. the constructor has finished), then it will encounter an undefined state in C and things might break. And since you can't know in A what the overwritten foo() expects, you get a warning.