Instanceof In Conditionals

 foo(Customer cust) {
   if(cust instanceof Person) {
     // do person related stuff
   }
   else {
     // do organisation related stuff
   }
 }

See also: TypeCase

A code smell. You are asking an external entity (the JavaVirtualMachine or other provider of RunTimeTypeInformation) to tell you something that the object should know about itself. It causes problems in practice because it means that business-level code has to know about the class hierarchy of Customer objects, that is, it has to understand implementation rather than specification.

Instanceof in conditionals is a time to ReplaceConditionalWithPolymorphism. At the very least, put an abstract Boolean method with a meaningful name on the Customer class.

Likewise, now that we have JavaGenerics there is very little excuse for casting. Rather than this:

 foo(Customer cust) {
   if(cust instanceof Person) {
     ((Person)cust).getPersonalName();
   }
   else {
     ((Org)cust).getBusinessName();
   }
 }

I very much prefer this:

 abstract class Customer {
   boolean isPerson() { return false; } 
   boolean isOrg() { return false; } 
   Person asPerson() { throw new IllegalStateException(); } 
   Org asOrg() { throw new IllegalStateException(); } 
 }

 class Person extends Customer {
   boolean isPerson() { return true; }
   Person asPerson() { return this; }
 } 

Note that "asPerson" can be implemented however you like. Business code then becomes:

 foo(Customer cust) {
   if(cust.isPerson()) {
     // do some person calculations
     cust.asPerson().getPersonalName();
   }
   else {
     // do some business calculations
     cust.asOrg().getBusinessName();
   }
 }

See? The code is asking the object about itself, rather than having to know things about class hierarchy. Let's say we have a customer type Dog, which for various reasons we do not want inheriting from "Person", but which for purposes of calculation acts like a person.

 class Dog extends Customer {
   boolean isPerson() { return true; }
   Person asPerson() { return new UnmodifiablePerson("Fido"); }
 }

The main problem is that you need to use MeaningfulNames. And those names must be consistent across the app. A big issue is when you include a boolean function like "isAcceptable", and "acceptable" means completely different things in different modules.

---

The key problem I can see is that it forces your BaseClasses to know about what classes are derived from them. I've used architectures like this in CeePlusPlus, and it usually ends up being an extensibility nightmare; when a new derived class is added, the effects ripple all over the place. -- TimLesher

I agree that it couples the derived classes with the super class. But there are not many simple alternatives. The instanceof test is problematic from a maintenance point too so neither way is proferrable from this position. For more complex situations either MultipleDispatch is needed (seldom available) or a more elaborate pattern for dealing with differing details. One example is the RoleObjectpattern?.

---

Why would you ever need instanceof outside a debugger? Surely that is what PolyMorphism is for? What is this customer/personal name really for? Assume it's for a label, then:

 abstract class Customer {
   String getLabel();
 }
 class Person extends Customer {
   String getLabel() { return getPersonalName(); }
 }
 class Company extends Customer {
   String getLabel() { return getBusinessName(); }
 }

 foo(Customer c) {
   String label = c.getLabel();
 }

The "isPerson/asPerson" stuff is just replacing one problem with another.

(Also, unless these objects have more than one name, these methods should already be abstract and called "getName".)

I disagree. That a name is used as a label is a frontend issue and the meaning of a person name and organization name may differ significantly. Also you may have other labels for a person - how to distinguish these? Abstracting may help/simplify AccidentalComplexity in some cases. But in many such cases it is EssentialComplexity that will not go away this way and instead cause work-arounds to be handled later on.

---

I've used things like instance-of (usually DynamicCast, to be exact) to good effect in plugin architectures, resource pools, and other things along those lines. The examples presented so far on this page, I agree, should all be accomplished with plain old polymorphism. But instance-of still has some uses. -- MichaelSparks

For plugin, this is not a smell:

 do {
     String name = user.ask("What plugin do you want to try to load?");
     Object potentialPlugIn = dynamicMagic.createInstanceOfNamedClass(name);
 } while(!(potentialPlugIn instanceof PlugInInterface));
 PlugInInterface plugIn = (PlugInInterface) potentialPlugIn;
 socket.plugIn(plugIn);

...unless your language provides a different mechanism.

---

Why not use two different methods for the two different behaviours?

 foo(Customer cust) {
   // do organisation related stuff
 }

 foo(Person pers) {
   // do person related stuff
 }

---

See ReplaceConditionalWithPolymorphism, ReplaceTypeCodeWithClass, ReplaceTypeCodeWithStateStrategy, and perhaps UseEnumsNotBooleans

SeptemberZeroSix

ObjectOriented