8
votes
\$\begingroup\$

I have an abstract class which holds a hook method for a template method. Not all classes which extend this class will need to implement this method (in fact most will not, but a few will). As such I declared the method this way:

protected void applyExtraCriteria(Object extraCriteria) {

}

Should it instead be declared this way:

protected abstract void applyExtraCriteria(Object extraCriteria);

Which one would be considered better practice?

\$\endgroup\$
1
  • 3
    \$\begingroup\$ As a rule you really should never implement an empty method in any class. If you want the method available to consumers of the abstract classes inheritors, declare it abstract. If you don't want to implement it on an inheritor because it doesn't make sense, then it should most likely throw an InvalidOperationException in the implementation of that inheritor. Which means you may want your abstract class to have an public abstract bool ExtraCriteriaAllowed { get; }; so consumers can know whether or not to use the method. \$\endgroup\$ Commented Aug 25, 2011 at 0:51

5 Answers 5

8
votes
\$\begingroup\$

If you are really sure that an empty method body is okay for most sub-classes, you could follow the example of Swing listeners (e.g. WindowListener / WindowAdapter): Make the methods in your Foo class abstract, but provide an abstract class called FooAdapter extending Foo, which implements that methods with empty bodies. That way you document your intentions: The methods must be implemented, but by extending the FooAdapter you say that you want for most of them the default implementation (which happens to have empty method bodies).

Update: Of course, if you can use Java 8, you can write default methods in interfaces, hence an abstract class isn't needed.

\$\endgroup\$
1
  • \$\begingroup\$ Many frameworks have these Adapter classes, so I don't think that extra level of abstraction would be difficult for others on my team to grasp. \$\endgroup\$
    – Scott
    Commented Aug 26, 2011 at 12:26
10
votes
\$\begingroup\$

I would declare as required by all classes. It is easy enough to leave an empty method stub in the sub class.

\$\endgroup\$
3
votes
\$\begingroup\$

I agree with IAbstract's answer, but I want to expand upon it with my own 2 cents.

The abstract base should make no assumptions as to whether or not the derived children will "need" the method. Indeed, if it does make the assumption, assume the children absolutely need it.

In addition, providing an empty implementation in the abstract base would possibly lead to children believing there is, in fact, some default implementation that might actually do something. You might have children overriding the method with an empty implementation simply because they do not want anything done!

(I realize you may likely be the one writing both the base and the child, but I think it is best to mentally seperate your own roles. When you're writing the base, you're the provider. When you're writing the child, you're the consumer. As a provider, develop the base in a way that will make most sense to your consumers.)

Make the method abstract. Let the children decide what (if any) implementation there needs to be for each step of the algoritm, following in the spirit of the template method patterm.

\$\endgroup\$
0
3
votes
\$\begingroup\$

I think it's better to distinguish between two cases here:

  • For a template method I believe it's better to have empty virtual method in base class. If you don't need to add any functionality you just ignore this part, but you will add some code only if you need to. This is exactly your case.

  • For a public method I believe it's better to have abstract method in base class. It's very unusual situation that your public method does no-op and at the same time you will not forget to implement it.

\$\endgroup\$
1
vote
\$\begingroup\$

Only methods that required to be implemented in subclasses should not have a default implementation. Other methods, whose implementation in subclasses is optional, can have it. The providing a default implementation is a way to say that a custom implementation for a method is not required.

\$\endgroup\$

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