mercoledì 25 gennaio 2012

Avoid downcast!

Who has never written a piece of code like the following (pseudo Java)?


if( myObject instanceof BASE )
  ((BASE) myObject).doBaseBehavior();
else if( myObject instanceof DERIVED1 )
  ((DERIVED1) myObject).doDerivedBehavior1();
else if( myObject instanceof DERIVED2 )
  ((DERIVED2) myObject).doDerivedBehavior2();

It is called downcast and is the opposite of the Liskov Substitution Principle. The idea is that, at run-time, you have to invoke a specialized behavior through a generic base class. Now, read the sentence again and emphasize the words "specialized", "through" and "generic". Note the order of such words: it is not correct! While it is true that a specialized implementation should offer a generic behavior, this does not mean that you can change ordering to the words and obtain something that can work. "Hey, Java offers instanceof operator and C++ offers dynamic_cast"  - I hear you screaming that. And so what? Do you think that just because there is such an operator in your language you should use in such a way? Not at all!
What happens when you are going to add a new derived subclass? You have to add another branch to your selection statement. That is a really bad looking piece of code, and trust me, sooner or later, you will forget to add such a branch and find your code is buggy.
I've seen this code more and more in every kind of project, and it seems to be nested also in libraries, especially commercial ones (at least this is my experience). You have to avoid this kind of programming. Let consider again your problem and create more abstractions, since what you are really expressing with the above code is that a few instances are pretty the same except for a single behavior. So they are not the same, do not manage them as they could be considered the same, or generalize such behavior so that all instances have the same common interface or prototype and are free to implement it as they need. So that instead of downcasting an instance you are simply casting it at one of its generic interfaces:

if( myObject instanceof MyPrototype )
  ((MyPrototype) myObject).doBehavior();

Interestingly my experience is that this kind of programming template (I would not call it a pattern!) arises from a bad comprehension of the factory pattern: you have a factory that creates different instances of the same interface, but since your interface lacks of specialization and you don't have the control on which implementation is going to be created, you try to downcast it. Again, this is not the solution, or at least, it is a solution that could work in the short term, but that you need to refactor to have robust code.
Finally, please consider that while doing casting the language operators are usually smart enough to inform you about an error in the cast. For instance, the dynamic_cast operator returns 0 if the cast was impossible. Similarly the instanceof operator returns false if the object is null. Take advantage of this information to write better quality code!

Nessun commento: