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
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.

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...