Is it OK to call abstract method from constructor in Java?

Question

Let's suppose I have an abstract Base class that implements Runnable interface.

public abstract class Base implements Runnable {

  protected int param;

  public Base(final int param) {
      System.out.println("Base constructor");
      this.param = param;
      // I'm using this param here
      new Thread(this).start();
      System.out.println("Derivative thread created with param " + param);
  }

  @Override
  abstract public void run();
}

And here is one of a few derivative classes.

public class Derivative extends Base {

  public Derivative(final int param) {
      super(param);
  }

  @Override
  public void run() {
      System.out.println("Derivative is running with param " + param);
  }

  public static void main(String[] args) {
      Derivative thread = new Derivative(1);
  }

}

The point is that I want my Base class do some general stuff instead of copying it every time. Actually, it's running fine, the output is always the same:

Base constructor Derivative thread created with param 1 Derivative is running with param 1

But is it safe IN JAVA to start a thread calling the abstract method in constructor? Because, in C++ and C# it is unsafe in most cases, so far as I know. Thank you!

Solution 1

This code demonstrates why you should never call an abstract method, or any other overridable method, from a constructor:

abstract class Super {
    Super() {
        doSubStuff();
    }
    abstract void doSubStuff();
}

class Sub extends Super {
    String s = "Hello world";

    void doSubStuff() {
        System.out.println(s);
    }
}

public static void main(String[] args) {
    new Sub();
}

When run, this prints null. This means the only "safe" methods to have in a constructor are private and/or final ones.

On the other hand, your code doesn't actually call an abstract method from a constructor. Instead, you pass an uninitialized object to another thread for processing, which is worse, since the thread you're starting may be given priority and execute before your Base finishes its initialization.

Solution 2

Not a good idea since when run() is invoked, the Derivative object may not have been initialized. If run() depends on any state in Derivative, it can fail.

In your simple case it works. But then there's no point for the subclass. You can simply

public Base(final int param, Runnable action) {

  new Thread(action).start();

Solution 3

It's a very bad practice to call an abstract method from a constructor. Methods called from constructors should always be private or final, to prevent overriding.

See this link to a question here

Solution 4

Passing this out of the constructor is called "letting this escape from the constructor", and can lead to some particularly nasty and weird bugs, because the object may be in an inconsistent state.

This is especially the case when this is passed to another thread, as in this example. Due to the JVMs right to reorder statements within a thread, you can get undefined behaviour/state occurring.

View more solutions

Comments

[supercat]

Would there be anything improper about having a constructor call an abstract or virtual method whose contract specifies that it may be called from a constructor, must be safe to call from such a context, and may only call other methods that are similarly safe?

[Ryan Stewart]

@supercat: I suppose that depends on how much you trust the documentation, how much you trust others to follow such documentation, and how much you trust everyone who might ever extend such a class or any subclass of it to appropriately propagate or remember to refer to such warnings. That's quite a lot of trust, in my opinion. I prefer things that can be proven by automated tests, and that can't.

[supercat]

Fair enough. The biggest usage case I had in mind was for a situation where a derived class would be expected to override a method to return a constant which must be the same for all instances of each subclass, and which will be needed in the constructor and elsewhere, so a typical implementation would be int getWoozleForType() { return 23;}. Passing such a thing to the constructor and having it store it in an instance field seems a bit icky, and I can't think of any other approaches that seem inviting either.

[Admin]

I don't get how this fails? It looks fine to me. Obviously you're correct but I can't wrap my head around how inheriting an abstract method and defining what it does can print null?

[Vivek Chavda]

@finnrayment The issue is that when Super constructor (which is called before Sub is constructed) attempts to call doSubStuff(), the String s will not have been initialized yet.

[Admin]

@VivekChavda Ahhh! Yes. I see now. Very interesting indeed.

[nawfal]

Just in case someone is interested, this issue doesn't happen in C#, and it prints "Hello world". This is because in C# first field level inline initializations are first called before constructor. Of course if you initialize s in the base constructor, C# too has the same problem.