Since I do like refactoring I decided that I would document code patterns that I stumble upon.
Consider following code (Java). First snippet shows
It is uneasy for me to explain why this is not a good solution because it is so obvious to me. It's like explaining why anecdote is funny to someone who does not get it. But since I met such code in more than one project written by different people there must be something worth explaining.
First, look at better version of this code:
This rewrite eliminates runtime check, removes dependency of base class on it's subclasses and removes temptation of junior developers who will modify this code after you to insert more insanceof checks in same method. And after all, in Java subclassing is language's idiomatic way do do such things.
I have written what wanted on this case but it is hard to me to stop here, so I continue.
If I would do refactoring I would stop after this rewrite. But looking at the methods I would scent that something else needs improvement: call to "bark" suggests that it belongs not to absrtact animal but to a dog. So if indeed we have only two subclasses Cat and Dog, I would further rewrite the code as:
I would advocate using composition over subclassing if everything else is equal. So if after these changes there's no real code left in Animal I would convert it to interface:
They tell me "ship it" and here I stop.
Consider following code (Java). First snippet shows
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
class BogusAnimal { | |
void action() { | |
if (this instanceof Cat) meow(); | |
else bark(); | |
} | |
} | |
class Cat extends BogusAnimal { | |
} |
It is uneasy for me to explain why this is not a good solution because it is so obvious to me. It's like explaining why anecdote is funny to someone who does not get it. But since I met such code in more than one project written by different people there must be something worth explaining.
First, look at better version of this code:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
class Animal { | |
void action() { | |
bark(); | |
} | |
} | |
class Cat extends Animal { | |
@Override | |
void action() { | |
meow(); | |
} | |
} |
This rewrite eliminates runtime check, removes dependency of base class on it's subclasses and removes temptation of junior developers who will modify this code after you to insert more insanceof checks in same method. And after all, in Java subclassing is language's idiomatic way do do such things.
I have written what wanted on this case but it is hard to me to stop here, so I continue.
If I would do refactoring I would stop after this rewrite. But looking at the methods I would scent that something else needs improvement: call to "bark" suggests that it belongs not to absrtact animal but to a dog. So if indeed we have only two subclasses Cat and Dog, I would further rewrite the code as:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abstract class Animal { | |
abstract void action(); | |
} | |
class Dog extends Animal { | |
@Override | |
void action() { | |
bark(); | |
} | |
} | |
class Cat extends Animal { | |
@Override | |
void action() { | |
meow(); | |
} | |
} |
I would advocate using composition over subclassing if everything else is equal. So if after these changes there's no real code left in Animal I would convert it to interface:
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
interface Animal { | |
void action(); | |
} | |
class Dog implements Animal { | |
@Override | |
void action() { | |
bark(); | |
} | |
} | |
class Cat implements Animal { | |
@Override | |
void action() { | |
meow(); | |
} | |
} |
They tell me "ship it" and here I stop.