2010-12-30

Do not do this, please: this instanceof

Since I do like refactoring I decided that I would document code patterns that I stumble upon.
Consider following code (Java). First snippet shows
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:
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:
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:
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.

On security

My VPS recently got banned for spam which surprised me since none of my soft there sending email. So my first thoughts were that this is a...