Call-Super antipattern

[Philipp]

Hello,
I just read about the "Call-Super" antipattern
(http://en.wikipedia.org/wiki/Call_super) and I'm not completely
convinced about the "anti-pattern" property of this construct.

Suppose you have a class hierarchy which makes sense, something like:

Car extends Vehicle
Ford extends Car

and they all have a start method:
Why is it bad for each start to call the super method? (in case each one
adds the super's behavior)
In particular, a proposed solution in the wiki article is to use the
template method pattern. How does that improve the design? In a
multi-inheritance structure as this example, you will still have to
choose between replicating code (Copy-Paste antipattern) or adding
template methods (hooks) for all possible subclasses.

Thanks for your comments.
Philipp

public class Vehicle {
public void start(){
// unlockDoor();
}

public static class Car extends Vehicle {
@Override
public void start() {
super.start();
// startEngine();
}
}

public static class Ford extends Car {
@Override
public void start() {
super.start();
//switchRadioOn();
}
}
}

 

[Daniel Pitts]

Hello,
I just read about the "Call-Super" antipattern
(http://en.wikipedia.org/wiki/Call_super) and I'm not completely
convinced about the "anti-pattern" property of this construct.

Suppose you have a class hierarchy which makes sense, something like:

Car extends Vehicle
Ford extends Car

and they all have a start method:
Why is it bad for each start to call the super method? (in case each one
adds the super's behavior)
In particular, a proposed solution in the wiki article is to use the
template method pattern. How does that improve the design? In a
multi-inheritance structure as this example, you will still have to
choose between replicating code (Copy-Paste antipattern) or adding
template methods (hooks) for all possible subclasses.

Thanks for your comments.
Philipp

public class Vehicle {
public void start(){
// unlockDoor();
}

public static class Car extends Vehicle {
@Override
public void start() {
super.start();
// startEngine();
}
}

public static class Ford extends Car {
@Override
public void start() {
super.start();
//switchRadioOn();
}
}
}

<rant>
First, The "Ford is-a Car" is an example of a terrible design. A Car
has-a Make and has-a Model, Ford is a Make, FordModelT is a Model. Think
about it, would you want to maintain the class hierarchy that contains
RedHondaCivicWithSpoiler.
</rant>

Despite that, a framework should rely as little as possible on consumers
to "do the right thing". The right way to do it is to have an
protected overridable method that isn't expected to call super, and the
non-overridable public method that delegates to the appropriate.

In other words, the work-flow should be defined by the class higher in
the hierarchy, and the details by the lower.

 

[Kevin McMurtrie]

Hello,
I just read about the "Call-Super" antipattern
(http://en.wikipedia.org/wiki/Call_super) and I'm not completely
convinced about the "anti-pattern" property of this construct.

Suppose you have a class hierarchy which makes sense, something like:

Car extends Vehicle
Ford extends Car

and they all have a start method:
Why is it bad for each start to call the super method? (in case each one
adds the super's behavior)
In particular, a proposed solution in the wiki article is to use the
template method pattern. How does that improve the design? In a
multi-inheritance structure as this example, you will still have to
choose between replicating code (Copy-Paste antipattern) or adding
template methods (hooks) for all possible subclasses.

Thanks for your comments.
Philipp

public class Vehicle {
public void start(){
// unlockDoor();
}

public static class Car extends Vehicle {
@Override
public void start() {
super.start();
// startEngine();
}
}

public static class Ford extends Car {
@Override
public void start() {
super.start();
//switchRadioOn();
}
}
}

You should know about patterns and anti-patterns but don't get religious
about them. There are always valid exceptions. What's important is
realizing when code is heading towards a bad design so its evolutionary
course can be corrected early on.

There are valid reasons to call super methods. Maybe very few for
everyday coding, but they exist. I have an abstract JPEG file processor
that's used by adding layers of functionality to a hierarchy of generic
segment handlers. Subclasses need to call super methods if they aren't
handling the entire hierarchy. It executes efficiently and subclasses
contain 3 to 10 lines of code. I'd say it has no anti-pattern problem
at this time.

I wouldn't use super methods for the Car example. Driving locks doors
in the US but unlocks them in Germany. The base class is already
broken. Events may more accurately model the way car manufactures plug
in varying components to a standardized wiring harness.

 

[Philipp]

<rant>
First, The "Ford is-a Car" is an example of a terrible design. A Car
has-a Make and has-a Model, Ford is a Make, FordModelT is a Model. Think
about it, would you want to maintain the class hierarchy that contains
RedHondaCivicWithSpoiler.
</rant>

Obviously. But making up good examples is an art of its own...

Despite that, a framework should rely as little as possible on consumers
to "do the right thing". The right way to do it is to have an protected
overridable method that isn't expected to call super, and the
non-overridable public method that delegates to the appropriate.

In other words, the work-flow should be defined by the class higher in
the hierarchy, and the details by the lower.

While I understand and agree with the above, this does not exactly solve
my problem (or it is not clear to me how). The point is, that all
classes in the hierarchy are instantiable and all should be startable
using the start() method.

So making start() call a hook() method in the top class, which can be
overriden is only solving the problem for the first child level. For the
grand-child, the question remains: should grandChild.hook() call
child.hook() or should we make child.hook() call anotherHook(), which
can then be overriden by the grand-child class. Martin Fowler goes to
great length about this in
http://www.martinfowler.com/bliki/CallSuper.html but in the end, he
comes up with two possible solutions which are IMHO, less elegant than
calling super (as if that was totally forbidden).

The two solutions are:
- reimplement the hook-calling method (ie. start() ) in lower classes.
This is ugly IMHO as you need to know a lot about how the parent works
(and also copies code). Also you have to adapt the code in child
classes, if the parent class changes.
- Use a new hook method (as explained above) in each subsequent level of
class hierarchy. This makes the API at least as brittle because it is
not clear from the code point of view which method should be overriden.
Having doc saying "for this level, override stillAnotherHook() and leave
start(), hook() and anotherHook() alone" is not much better than having
doc saying "when overriding start(), call its super implementation"

Maybe I'm missing the point. And I totally agree that inversion of
control or template method are appropriate for frameworks (although the
example given by Martin Fowler is precisely about the JUnit framework).

Phil

 

[Daniel Pitts]

Obviously. But making up good examples is an art of its own...


While I understand and agree with the above, this does not exactly solve
my problem (or it is not clear to me how). The point is, that all
classes in the hierarchy are instantiable and all should be startable
using the start() method.

Is there a "good" reason its a hierarchy? Can you refactor to use
Composition instead of Inheritance? If you can limit to one base class,
and THEN use the start/hook approach (not the best names, IMO, but thats
another thread ^_^). In this approach, start() would be final, and call
hook() on itself, and start() on all its composed objects (if you have a
Collection, this is a simple for-each).

So making start() call a hook() method in the top class, which can be
overriden is only solving the problem for the first child level. For the
grand-child, the question remains: should grandChild.hook() call
child.hook() or should we make child.hook() call anotherHook(), which
can then be overriden by the grand-child class. Martin Fowler goes to
great length about this in
http://www.martinfowler.com/bliki/CallSuper.html but in the end, he
comes up with two possible solutions which are IMHO, less elegant than
calling super (as if that was totally forbidden).

The reason its dangerous is that it couples your base class and derived
classes more than they should be in a flexible design.

The two solutions are:
- reimplement the hook-calling method (ie. start() ) in lower classes.
This is ugly IMHO as you need to know a lot about how the parent works
(and also copies code). Also you have to adapt the code in child
classes, if the parent class changes.
- Use a new hook method (as explained above) in each subsequent level of
class hierarchy. This makes the API at least as brittle because it is
not clear from the code point of view which method should be overriden.
Having doc saying "for this level, override stillAnotherHook() and leave
start(), hook() and anotherHook() alone" is not much better than having
doc saying "when overriding start(), call its super implementation"

That's what creating final methods are for, they can't be accidentally
overridden.

Maybe I'm missing the point. And I totally agree that inversion of
control or template method are appropriate for frameworks (although the
example given by Martin Fowler is precisely about the JUnit framework).

I think the main point is to try to keep as much to possible to having
one class have only one responsibility, if you're class has to take care
of there geriatric parent, they aren't going to have as much time for
themselves . IOW, it makes it more "heavy" psychologically to
realize that you have to call "super" or risk breaking the flow.

Sometimes it makes sense to call super, but when most of your whole
hierarchy does it, that indicates an opportunity to at least *consider*
a different design pattern.

It may be tricky to find the other pattern that works, from what design
you already have. Nobody says big refactoring is easy. It is more than
worth it when you realize how much easier it is to extend the new design.

 

[Tom Anderson]

While I understand and agree with the above, this does not exactly solve my
problem (or it is not clear to me how). The point is, that all classes in the
hierarchy are instantiable and all should be startable using the start()
method.

So making start() call a hook() method in the top class, which can be
overriden is only solving the problem for the first child level. For the
grand-child, the question remains: should grandChild.hook() call
child.hook() or should we make child.hook() call anotherHook(), which
can then be overriden by the grand-child class.

Clearly, what Java needs is a sub keyword. Like super, but it goes the
other way.

Uh, thinking about it, maybe not.

But a language feature which means something like "when you send this
message to an object, invoke all the matching methods in the hierarchy,
not just the most-overriding one" would do it. I'm not aware of any
language having a feature like that; i think you could get the effect in
python using a metaclass, and you could probably do the same in any other
language that supports deep metamagic, like LISP, but nothing lets you do
it directly.

One of my rules of thumb is to look for ways to replace inheritance with
composition. So, how about this, using a pumped-up version of the Type
Object pattern (or is it really Strategy?):

/* our sort-of Type Object */
public abstract class VehicleType {
public final VehicleType base ; // used to mimic inheritance hierarchy
protected VehicleType(VehicleType base) {
this.base = base ;
}
// sort-of Template Method pattern here
public void start() {
handleStart() ;
if (base != null) base.start() ;
}
public abstract void handleStart() ;
}

/* the instances of the Type Object - which are actually singletons of
different classes, defined as anonymous classes */

public static final VehicleType VEHICLE = new VehicleType (null) {
public void handleStart() {
unlockDoors() ;
}
} ;

public static final VehicleType CAR = new VehicleType (VEHICLE) {
public void handleStart() {
startEngine() ;
}
} ;

public static final VehicleType FORD = new VehicleType (CAR) {
public void handleStart() {
switchRadioOn() ;
}
} ;

/* our vehicle class */
public class Vehicle {
public final VehicleType type ;
public Vehicle(VehicleType type) {
this.type = type ;
}
public void start() {
type.start() ;
}
}

(note that i haven't tried to compile this, so please excuse any syntax
errors; i hope the intent is clear)

Is this icky?

In reality, you'd want VehicleType.start to take a Vehicle as a
pseudo-this parameter, so it can operate on the vehicle being started.

A problem arises when you want to have more than just start() - maybe
stop(), accelerate(), brake(), etc, since then the template methods in
VehicleType all have to include the boilerplate to walk the type object
hierarchy, which means duplication of code. If we had higher-order
functions, the walking could easily be refactored, but we don't. Thus,
we'd have to use a bit of Visitor pattern:

/* a visitor type, of sorts */
public abstract class VehicleAction
{
public abstract void apply(VehicleType type) ;
}

/* modified Type Object with a visitation method */
public abstract class VehicleType {
public final VehicleType base ; // used to mimic inheritance hierarchy
protected VehicleType(VehicleType base) {
this.base = base ;
}
// sort-of Template Method using the sort-of Visitor
public void do(VehicleAction action) {
action.apply(this) ;
if (base != null) base.do(action) ;
}
public abstract void handleStart() ;
public abstract void handleStop() ;
}

/* concrete visitors, again as singleton instances of anonymous classes */

public static final VehicleAction START = new VehicleAction() {
public abstract void apply(VehicleType type) {
type.handleStart() ;
}
}

public static final VehicleAction STOP = new VehicleAction() {
public abstract void apply(VehicleType type) {
type.handleStop() ;
}
}

/* aaand finally ... */
public class Vehicle {
public final VehicleType type ;
public Vehicle(VehicleType type) {
this.type = type ;
}
public void start() {
type.do(START) ;
}
public void stop() {
type.do(STOP) ;
}
}

This is the kind of thing that gives OOP a bad name, isn't it?

tom

 

[Christian]

Clearly, what Java needs is a sub keyword. Like super, but it goes the
other way.

Uh, thinking about it, maybe not.

But a language feature which means something like "when you send this
message to an object, invoke all the matching methods in the hierarchy,
not just the most-overriding one" would do it. I'm not aware of any
language having a feature like that; i think you could get the effect in
python using a metaclass, and you could probably do the same in any
other language that supports deep metamagic, like LISP, but nothing lets
you do it directly.

I think that would be the real antipattern.

when you call super in a class that you override that is ok and often
needed. As you are having a good look at that class and therefore know
if a method needs to call super or not to keep a consistent state for
itself.

If one allowed calling all methods of all super type that would make you
responsible for all objects in the hirarchie ..and like that breaking
the encapsulation the object you extend represents.

Christian

 

[Tom Anderson]

I think that would be the real antipattern.

when you call super in a class that you override that is ok and often needed.
As you are having a good look at that class and therefore know if a method
needs to call super or not to keep a consistent state for itself.

If one allowed calling all methods of all super type that would make you
responsible for all objects in the hirarchie ..and like that breaking
the encapsulation the object you extend represents.

I look at it as making each class in the hierarchy responsible for making
sure that their method can be called by subclasses. I don't see that it
would break encapsulation. Indeed, by doing it declaratively, rather than
by requiring super calls everywhere, you reduce the burden of
responsibility on subclasses, and reduce the coupling between levels.

It would need to be done in a form that was obvious to the programmer,
though, so they couldn't fail to notice that it was happening. Perhaps by
introducing a keyword in the declaration, and requiring that it be in any
'overriding' declaration as well, so if a programmer didn't realise, and
omitted it, the compiler would tell them. So:

public class Vehicle
{
public super void start()
{
unlockDoors() ;
}
}

public class Car
{
public void start() // not declared 'super'
{
startEngine() ;
}
}

Would be a compile-time error.

I should add that i'm not seriously advocating this as a feature for java,
or any other language. I just think it's an interesting idea to kick
around!

tom

 

[Philipp]

>
Is there a "good" reason its a hierarchy? Can you refactor to use
Composition instead of Inheritance? If you can limit to one base class,
and THEN use the start/hook approach (not the best names, IMO, but thats
another thread ^_^). In this approach, start() would be final, and call
hook() on itself, and start() on all its composed objects (if you have a
Collection, this is a simple for-each).

In my structure, it is not too complicated to refactor inheritance by
composition. All methods of interest are defined in interfaces anyway,
so you just get a ton of delegate methods (which luckily Eclipse can
generate for me).
One problem which I came accross while trying to switch from inheritance
to composition is how to handle correctly the this pointer.

In the following, as a naming convention I'll call "container" the class
which contains an instance of the "enclosed" class. Both implement some
interface and container delegates the interface methods to enclosed.

In my structure, the enclosed.start() method described in the original
post is actually registering enclosed as a listener to some event source
by passing the this pointer. Evidently when using composition, the
events should not be handled by enclosed, but by container. A possible
solution is to *also* register container as a listener and just drop
events dispatched to enclosed. Is this a nice way of doing things? Is
there some pattern which addresses this issue?

BTW, in this scenario, I'll end up calling enclosed.start() from
container.start() which is pretty equivalent to the super call we were
discussing earlier (isn't it?).

What do you think?

It may be tricky to find the other pattern that works, from what design
you already have. Nobody says big refactoring is easy. It is more than
worth it when you realize how much easier it is to extend the new design.

Yep I can see that...

Phil

 

[Andy Dingley]

I just read about the "Call-Super" antipattern
(http://en.wikipedia.org/wiki/Call_super) and I'm not completely
convinced about the "anti-pattern" property of this construct.

Why is it bad for each start to call the super method?

It isn't. As the limiting case, it's obviously better than when the
over-ridden method _doesn't_ call the super method, and it ought to
have done so!.

The problem isn't that architectures where subclasses _do_ correctly
call super methods are at all bad. If you get it right, then it works
fine. The problem is that it's not always obvious that they're doing
this correctly. Using the call-super antipattern (which is massively
common) has the drawback that superclasses now _require_ their
subclasses to behave in a partcular way (by calling super methods when
needed). If the subclass doesn't comply with this, things break.

A better approach is the use of the template pattern. In this case the
_superclass_ gets to define what should be called and when, and it
remains in full control of whether this continues to be done when the
object's type is that of a subclass. The subbclass can still over-ride
what needs it, but it no longer has the scope to accidentally break
things by "forgetting" to support the implied requirements upon it,
such as callign the right super methods.

A corrolary of this is that a superclass using the template pattern
can also be refactored later to change this behaviour. Try doing that
with call-super though, when it has already been subclassed a bunch of
times, even into 3rd party code you haven't even seen, and you want to
then enforce the corresponding change to the called super methods onto
that bunch of subclasses.