I’m currently working my way through Michael Feathers Working Effectively With Legacy Code. Damn good book, BTW. Anyway, this was this section on “programming by difference” that got my thinking while reading it.
In this section, Michael describes adapting a
MessageForwarder. The existing implementation has a method,
getFromAddress(), which says who the message that is being forwarded is from. Michael wants to adapt the class, without changing the client (significantly), so he creates a subclass,
AnonymousMessageForwarder, that overrides the
All through this, my mind was screaming “No, don’t do it like that!”. What I wanted to see was this sequence of steps:
* Make a subclass of
MessageForwarder, called something like
* Move the implementation of
getFromAddress() down to the
SignedMessageForwarder, leaving the version in
* Then create the
(This slots in _after_ the tests are written, of course.😉
Then, about 6 pages later, Michael did exactly this (except that he called the other subclass an
AddresPreservingForwarder, which proves that he comes up with better names than I do). *phew* I felt better reading that. However, I disagreed with why he did it.
Michael invoked the Liskov Substituion Principle, which I’ve rabbited on about before, so I’m not going to do it again. I don’t think the LSP is the most valid reason for doing this. IMO, he gave the best reason in the title of the section: Programming by Difference.
When you have a class hierarchy, it is easy to see differences between classes at the same level. You can easily scan the class signatures and see the methods that each have overriden from their respective parent. It’s not as simple to see the differences between a parent and a subclass. If you start from the parent class, you may not even realise that the subclass exists.
What I like to try and do when using implementation inheritance (as opposed to type inheritance) is to keep methods designed for inheritance extremely simple. In the superclass, I strive for these rules:
- If the method belongs in the super, make it final. Use the Template Method pattern to give hooks in for subclasses.
- If the intended subclass needs to define the method, make it abstract.
- If an implementation is optional (as is often the case with the Template Method pattern), provide an empty implementation.
- Override concrete methods only to limit behaviour, not expand behaviour.
- Above all else: avoid having complex methods that are completely overriden by a subclass. This just confuses things no end.
- Finally: don’t have protected variables!
These rules tend to give me inheritance trees of a fairly uniform depth. It also usually gives me trees that are, in Michael Feather’s terms, “normalised” – that is, class hierarchies where a class only has one implementation of any given method between itself and its parents. This is a very powerful technique for determining differences between classes.
It’s also a corollary of the “Single Responsibility Principle”, which Michael elaborates on as well – if a class has some behaviour that a subclass wants, but also has behaviour the subclass doesn’t want (and will remove by overriding), then the class probably hasn’t had the responsibilities broken down far enough.
I find people tend to either be abusers of inheritance, or have been so scarred by it they swing entirely the other way, practically banning it. Personally, I find it powerful when not abused, and the rules listed above are some of the ways I try to not abuse it. Another is never to have a class hierarchy that makes me want to draw it on a scroll instead of a piece of paper.