PLEASE SEE THE UPDATES BELOW, I didn't realize that my issue was with the Template Method Pattern.
------------------------------------------------------------------------
Original Post:
I coded up a smell today, I had an abstract class who's public methods were calling the abstract implementations. Here's kind of what it looked like:
public abstract class Foo {
abstract SomeObj foo();
abstract SomeObj bar();
public void somethingFooDoes() {
SomeObj a = foo();
SomeObj b = bar();
//do some stuff with a + b
}
}
How did I get here you might ask. Well, there were going to be two implementations of Foo, a prod implementation and a sandbox implementation. I started off with Foo as an interface and began coding up the Prod + Sandbox impls. It turned out that the prod and sandbox implementations had really similar structures. So, I made Foo an abstract class and moved the common stuff in there. A few more refactorings and Foo ended up looking like it does above. It then dawned on me, Foo was really using different strategies to get at the SomeObjs, I refactored to the strategy pattern:
public class Foo {
private final SomeObjProducer someObjProducer;
public Foo(SomeObjProducer someObjProducer) {
this.someObjProducer = someObjProducer;
}
public void somethingFooDoes() {
SomeObj a = someObjProducer.foo();
SomeObj b = someObjProducer.bar();
//do some stuff with a + b
}
}
Now Foo has become concrete and is configurable with the different strategies for producing SomeObjs. To me this code is much more flexible and it now has collaborators. Where in the Abstract implementation it was kind of collaborating with itself (yuck!)
What's the moral of the story? If you have an abstract class and you find yourself calling abstract methods in your concrete methods (whoa that's a mouthful) consider refactoring to the strategy pattern.
UPDATE: a co-worker correctly pointed out that the first code snippet of Foo was the Template Method Pattern. Maybe it's too broad a statement, but it seems that anytime you have a template method pattern you could refactor it into the strategy pattern. Taking your inheritance based code and refactoring it into composition based code. Context is king, but these days I certainly favor composition over inheritance.
UPDATE AGAIN: I received some good complaints regarding my code sample.
Imagine using the more standard Game example from wikipedia (but simplified for brevity)
public abstract class Game {
public void playOneGame() {
while(!endOfGame()) {
makePlay();
}
}
public abstract boolean endOfGame();
public abstract void makePlay();
}
public class GamePlayer {
private Game game;
public GamePlayer(Game game) {
this.game = game;
}
public void playOneGame() {
while(!game.endOfGame()) {
game.makePlay();
}
}
}
