Tuesday, June 26, 2007

Calling super() considered harmful?

Had an interesting discussion yesterday about how to solve the following problem:

  • You have a class (in our case, it was a Spring MVC controller) which has some dependencies injected, and which you, being a good defensive programmer, want to check that all the needed dependencies are present.
  • Since this is a Spring app, it's ok to use the InitializingBean's afterPropertiesSet method somewhere in here.
  • We want to derive some subclasses from the base class that introduce additional dependencies that we want checked, but we want to continue checking the superclass's dependencies.

How would you do this?

Option A. Template method. This would look as follows:

public class Parent implements InitializingBean {
  private ParentDep parentDep;
  public void afterPropertiesSet() {
    if (parentDep == null) {
      throw new IllegalStateException();
    }
    checkChildConfig();
  }
  public void checkChildConfig() { }
  ...
}

public class Child extends Parent {
  private ChildDep childDep;
  @Override void checkChildConfig() {
    if (childDep == null) {
      throw new IllegalStateException();
    }
  }
  ...
}

Pros: (a) parent checks parent configs, child checks child configs. (b) parent configs are always checked, unless the child decides to do something like override afterPropertiesSet. (c) no use of super. (d) This is the one Martin Fowler likes.

Cons: (a) parent has to be aware it will be subclassed. (b) this gets considerably awkward with more than one level of inheritance (i.e. what if there's a GrandChild class?): Parent may need to be modified, or Child may have to override the afterPropertiesSet method of the Parent to cover this case.

Option 2. Just use super.

This one looks this way:

 public class Parent implements InitializingBean {
   private ParentDep parentDep;
   public void afterPropertiesSet() {
     checkConfig();
   }
   public void checkConfig() {
     if (parentDep == null) {
       throw new IllegalStateException();
     }
   }
   ...
 }

 public class Child extends Parent {
   private ChildDep childDep;
   @Override public void checkConfig() {
     super.checkConfig();
     if (childDep == null) {
       throw new IllegalStateException();
     }
   }
   ...
 }

Pros: (a) Parent checks parent configs, child checks child configs. (b) Parent doesn't care if it gets subclassed, or what the depth of the inheritance hierarchy is. In particular, Parent probably doesn't have to be modified with the addition of more hierarchy.

Cons: (a) Child has to remember to call super.checkConfig in its own checkConfig; if not, then the parent dependencies won't get checked. (b) Martin Fowler will look on you with disdain for using super.

It strikes me that if I'm going to inherit from a class (particularly from one where I don't have access to the original source code of the Parent, or where the coder of the Parent didn't totally contemplate that it would be subclassed), I'm probably going to end up overriding some of the parent's methods here or there, and using the parent's methods in other places. Why is this any different? Part of successfully subclassing a class would mean not breaking the "contracts" established by the Parent, and super gives you a nice, easy way to say "oh yeah, and check what he was checking too."

Opinions?

0 comments: