StrangeWill
StrangeWill

Reputation: 2136

Calling method from constructor

Excuse any minor syntax errors or whatnot, I'm experiencing this with a Jitsi module and not being super familiar with Java want to confirm what is going on and why and how it should be fixed.

 public abstract class A
{
  public A()
  {
    this.load();
  }

  protected void load()
  {

  }
}

public class B extends A
{
  private String testString = null; 

  public B()
  {
    super();
  }

  @Override
  protected void load()
  {
    testString = "test";
  }
}

The application is doing this when creating an instance of the class B using a load class by name method:

Is this expected Java behavior? What could cause this? It's a Java 1.6 application running on the 1.7 JDK.

Upvotes: 23

Views: 17565

Answers (2)

Thomas W
Thomas W

Reputation: 14164

This is a common problem-pattern with initialization-on-construction, and can frequently be found in infrastructure code & home-made DAOs.

The assignment to 'null' is unneeded & can be removed.

If that's not enough as a quick patch, then: Move all the post-construction init to a separate method, and wrap it all in a "static method" pseudo-constructor.

And if you're doing DAO stuff, it's really good to distinguish between "load" and "create", since these are completely different instantiations. Define separate "static constructor" methods & perhaps separate internal inits, for these.

abstract public class A {
    protected void initAfterCreate() {}
}

public class B {

    @Override
    protected void initAfterCreate() {
        this.testString = "test";
    }

    // static constructors;
    //     --        
    static public B createB() {
        B result = new B();
        result.initAfterCreate();
    }
}

Demonstrating load/create separation for a DAO:

public class Order {
    protected int id;
    protected boolean dbExists;

    static public load (int id) {
        Order result = new Order( id, true);
        // populate from SQL query..
        return result;
    }
    static public create() {
        // allocate a key.
        int id = KeyAlloc.allocate( "Order");
        Order result = new Order( id, false);
    }

    // internal constructor;  not for external access.
    //
    protected Order (int id, boolean dbExists) {
        this.id = id;
        this.dbExists = dbExists;
    }
}

Upvotes: 5

Rohit Jain
Rohit Jain

Reputation: 213203

Is this expected Java behavior?

Yes.

What could cause this?

Your invocation of non-final overridden method in non-final super class constructor.

Let's see what happens step-by-step:

  • You create an instance of B.
  • B() calls super class constructor - A(), to initialize the super class members.
  • A() now invokes a non-final method which is overridden in B class, as a part of initialization.
  • Since the instance in the context is of B class, the method load() invoked is of B class.
  • load() initializes the B class instance field - testString.
  • The super class constructor finishes job, and returns (Assuming chaining of constructor till Object class have been finished)
  • The B() constructor starts executing further, initializing it's own member.
  • Now, as a part of initilization process, B overwrites the previous written value in testString, and re-initializes it to null.

Moral: Never call a non-final public method of a non-final class in it's constructor.

Upvotes: 60

Related Questions