A Funny Java Flavoured Look at the World

Thursday, May 18, 2006

Design for Extension

I have checkstyle turned on in eclipse and it's always moaning about something, especially when I am in the legacy code jungle. Normally I don't mind it because I change a few things and try and put things on the right path but this rule I found weird and couldn't make go away.  It is the rule called
 
"Design for Extension"
 
this is the text describing the rule
 
"Checks that classes are designed for extension. More specifically, it enforces a programming style where super classes provide empty "hooks" that can be implemented by subclasses. The exact rule is that nonprivate, nonstatic methods of classes that can be subclassed must either be a) abstract or b) final or c) have an empty implementation.  Rationale: This API design style protects superclasses against being broken by subclasses. The downside is that subclasses are limited in their flexibility, in particular they cannot prevent execution of code in the superclass, but that also means that subclasses cannot corrupt the state of the superclass by forgetting to call the super method."
 
Normally I don't mind checkstyles rules because for the most part they are getting you to do is make you code better by making variables that aren't going to change final, making me put JavaDoc notes in now so I don't end up just before we release the software having to go back and write out the all the JavaDocs for the classes I have changed, which is a really boring task if there are loads of them.
 
Anyway back to the Design for Extension rule.  I realise you don't have to implement all the rules checkstyle has and there are some I have turned off but I was wondering whether I should turn this rule off or not.  This rule seems a bit over the top.  I suppose I think this is a bit over the top because our code is only used internally I don't feel I have to protect against people extending this class and then overriding this method and breaking the code.  If they do that then it's there own stupid fault
 
but...
 
If I adhered to this rule then this couldn't happen, it would stop a bug occurring which might be done by accident.  The interesting thing about this rule is now that I thought about it, it does have some merit, it is a good ideal.  Alot of the code that is written doesn't design classes to such an extent and there are many legacy classes that are written without thought about whether they are going to be extended or not and probably very unlikely they would be extended.  So when this rule pops complaining it seems slightly absurd, why am worried about people extending this class and overriding this method.  The other problem I have is that although I like writing final variables and trying to make classes final I'm not so keen on making methods final.  I don't really know why this is but if perhaps I do want to extend this class do I want this method final and not.
 
It's interesting with the checkstyles rules they really help you to decide how a class should work and you spend more time thinking of the design of the class.  I started writing this blog entry with the idea of saying that I didn't like this rule but after looking at the idea behind the rule I actually am in favour of it now.  So I think I will leave the rule on just because it helps me think about the design of the class
 
 

2 Comments:

  • The rule is good, and started me thinking a couple of years ago.

    If you don't design for extension, then you are not going to have to think about the possibilities of problems caused by a subclasser.

    Fine, if someone subclasses badly, that's their problem, but it's not. It's your problem too, because the resulting object is a mixture of your class and theirs, and hence you will probably have to cooperate with them to sort out the issue.

    Another way of saying the same thing is that you should carefully consider ALL the stuff that you expose to other code, even if it's you who writes it.

    More radically, I'd encourage names for instantiable classes to be package-private, or even better, for instantiable classes to be anonymous inner classes.

    I know, I know, this prevents subclassing. That's one of the ideas. Mainly, it encapsulates well.

    A good argument against subclassing is that it makes testing individual types impossible.

    Also, it is high (or concrete) coupling, i.e., coupling to behaviour, instead of coupling to an interface. Other forms of concrete coupling are better, e.g., calling a static utility method, but ideally there would be minimal concrete coupling.

    Concrete coupling makes unit testing (not all automated tests are unit tests) impossible, assuming that one class is one unit, as you cannot substitute in mock implementations, except with tricks like bytecode manipulation.

    By Blogger Ricky Clarkson, at Thu May 18, 02:59:00 pm 2006  

  • very constructive comments, thanks a lot. I think the rule is correct because I believe we should spend more time trying to protect our classes from misuse or bugs.

    Hiding things through access modifiers or making methods final is one way of doing this.

    By Blogger The Hosk, at Fri May 19, 10:44:00 pm 2006  

Post a Comment

<< Home