Do never invoke overriding methods in constructors

I came across these 2 methods. They both where used in completely different classes. But I stuck because they were slightly similar (I have simplified the code of the following examples)

public class OtherClass{
   protected OtherMyModel createOtherMyModel() {
       OtherMyModel model = new OtherMyModel ();
       model.initDefaultSearchValues();
       model.doSomething();
       return model;
   }
}
public class SomeClass{
   protected MyModel createMyModel() {
       MyModel model = new MyModel ();
       model.initDefaultSearchValues();
       return model;
   }
}

When I examined the classes MyModel and OtherMyModel I realized they were inherited classes of AbstractMyModel. So as they both always called initDefaultSearchValues() after their initialization, I thought why not calling that method in the constructor? No sooner said than done. I have to mention that Tests were written for those methods and they worked just fine. But after changing the code by calling initDefaultSearchValues() in the constructor of the superclassa a NullPointerException occured. Take a second and ask yourself why this exception could have occurred.

DSC_0676Klein

I did forget an important rule when using inheritance: “Constructors must not invoke overridable methods, directly or indirectly” ([1], p.89f). Why? Let’s look  at one of the overridden methods.

public class MyModel extends AstractMyModel{
   private final Set<Class<? extends IIpsObjectPart>> searchedClazzes = new HashSet<Class<? extends IIpsObjectPart>>();
   @Override
   protected void initDefaultSearchValues() {
       if(searchedClazzes.size()){ //This causes the problem
       ...
       }
}

When instantiating a new MyModel, the constructor is first called. We have to take some things into account [2]:

  • As there is no constructor explicitly defined the default constructor is called.
  • When using inheritance the superclass constructor should always be invoked.
  • If a constructor does not explicitly invoke a superclass constructor, the Java compiler calls automatically the default constructor of the superclass
  • The superconstructor has to finish its calls, afterwards the variables of the subclass are instantiated and the constructor of the subclass is called

So when initiating MyModel  the member variables of the superclass (AbstractMyModell) are instantiated and the super constructor is called if declared. Then the constructor in the superclass calls the implementation of initDefaultSearchValues(). In that moment all member variables of the subclass MyModel are not yet instantiated. That applies to the variable searchedClazzes too. They will be instantiated when the constructor of the super class is finished. Therefore the compiler goes straight to initDefaultSearchValues, which uses searchedClazzes, which in that particular moment is null. And then the NullpointerException is thrown.

So remember do NEVER EVER use overriding methods in constructors. It may cause serious errors. Better safe than sorry. Therefore bear in mind: Test Test Test. I wouldn’t have recognized my error if I didn’t test. For classes using inheritance do test your subclasses. Abstract classes can’t be instantiated. So you have to use your concrete fully implemented class for tests. Writing testcases for different subclasses are even a great opportunity for code refactoring because you may detect duplicate code and can easily pull that code up to the superclass. Another source for refactoring within inherited classed are the declared variables.  Variables marked as protected can be used by all subclasses, when working on all subclasses -better: when testing all subclasses- you may recognize that it isn’t needed and you can make it private

Don’t neglect testing -it sure is not a waste of time, it’s an opportunity to make stable code.

References

[1] Bloch, Joshua (2008): Effective Java Second Edition, Addison Wesley

[2] http://docs.oracle.com/javase/tutorial/java/IandI/super.html

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s