Call-Super antipattern

  • Follow


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();
		}
	}
}

0
Reply sicsicsic (164) 5/6/2008 9:56:31 AM

Philipp wrote:
> 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.


-- 
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
0
Reply newsgroup.spamfilter (920) 5/6/2008 6:20:47 PM


In article <1210067774_4303@sicinfo3.epfl.ch>,
 Philipp <sicsicsic@freesurf.ch> wrote:

> 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.

-- 
Block Google's spam and enjoy Usenet again.
Reply with Google and I won't hear from you.
0
Reply mcmurtri (747) 5/7/2008 7:02:33 AM

Daniel Pitts wrote:
> Philipp wrote:
>> 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>

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
0
Reply sicsicsic (164) 5/7/2008 8:41:24 AM

Philipp wrote:
> Daniel Pitts wrote:
>> Philipp wrote:
>>> 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>
> 
> 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.
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.
-- 
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
0
Reply newsgroup.spamfilter (920) 5/7/2008 2:55:23 PM

On Wed, 7 May 2008, Philipp wrote:

> Daniel Pitts wrote:
>> Philipp wrote:
>>> 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.
>>
>> 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.

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

-- 
Ensure a star-man is never constructed!
0
Reply twic (2083) 5/7/2008 4:03:47 PM

Tom Anderson schrieb:

> 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
0
Reply fakemail9312 (192) 5/7/2008 4:41:25 PM

On Wed, 7 May 2008, Christian wrote:

> Tom Anderson schrieb:
>
>> 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.

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

-- 
Ensure a star-man is never constructed!
0
Reply twic (2083) 5/7/2008 5:38:35 PM

Daniel Pitts wrote:
> Philipp wrote:
>> Daniel Pitts wrote:
>>> Philipp wrote:
>>>> 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.
>>> <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>
 >
> 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
0
Reply sicsicsic (164) 5/8/2008 7:12:31 AM

On 6 May, 10:56, Philipp <sicsic...@freesurf.ch> wrote:
> 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.
0
Reply dingbat (840) 5/8/2008 3:34:04 PM

9 Replies
26 Views

(page loaded in 0.161 seconds)


Reply: