23

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:

  • Calls overridden load() in class B
  • Initializes variables (calls "private string testString = null" according to debugger), nulling them out.

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

0

2 Answers 2

60

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.

2
  • Awesome, thanks. I'll investigate how other parts of the project operate, they must be doing it right and this is the outlier, so I'll follow the rest of the application's behavior. Commented Aug 9, 2013 at 0:37
  • Does this mean that if we do call an instance method from a constructor we are calling a half-cooked object's method ?
    – Adithya
    Commented Oct 19, 2016 at 17:13
5

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;
    }
}
3
  • load() is a method typically used by modules to pull their configuration from .properties files, I've touched base with the Jitsi dev mailing list and we'll see what their recommended approach is to this to keep consistency in their code (not all modules load properties, and I got tired of looking... have to file a bug on their list anyway). Commented Aug 9, 2013 at 1:05
  • Okay, so it's not DAO stuff. That will make it simpler -- should just be one initialization path then. initAfterCreate() is the only fundamentally reliable solution, and preferably wrap that in a static factory method.
    – Thomas W
    Commented Aug 10, 2013 at 3:30
  • Why a downvote? Sharing a lot of actual experience of init-on-construction tangles -- the practical circumstances in which they arise, and effective patterns ('static constructor') to resolve them.
    – Thomas W
    Commented Sep 2, 2013 at 1:18

Not the answer you're looking for? Browse other questions tagged or ask your own question.