What is this code smell called?

Question

I think of this as broken OOP. It's starting to grow like mold in our code base and I need to start talking with people about cleaning it up. Is there a common name for it?

public void eat(Fruit fruit) {

    if (fruit instanceof Banana) {
        ((Banana) fruit).peel();
    }
    else if (fruit instanceof Pineapple) {
        ((Pineapple) fruit).slice();
    }

    // ... etc ...

    else if (fruit instanceof Strawberry) {
        // n/a ;-)
    }

    fruit.eat();
}

Answer 1

It's called a "type switch", like a switch statement on the type of an object.

Another question that will probably be of interest to you.

Answer 2

"Replace conditional with Polymorphism" is the name of the refactoring you are thinking of. Can't think of the name of the smell.

It is discussed in Martin Fowlers book Refactoring: Improving the Design of Existing Code.

Answer 3

I don't know that it has a name (Type Switch CS?), but there's an interesting discussion of it (in Java) here.

Answer 4

Faked polymorphism?

Answer 5

It is called downcasting. link1 link2

Answer 6

Its called, "Time to go buy a book and learn some design patterns". The Head First Design Patterns first chapter covers how to avoid this. Fruits extend Fruit, Prepare behaviors implementing PrepareBehavior. Inside your Fruit base class you could have (compose) whatever PrepareBehavior matches the fruit. (Strategy Pattern)

Answer 7

I know what you're trying to ask, but the real problem is the design of your eat method and the architecture of your object hierarchy. There is no way to "eat a Fruit" in your architecture because each derived type of fruit has a different way of preparing itself to be eaten (one needs to be sliced, one needs to be pealed). The notion of eat (myFruit) itself is flawed.

You should change Fruit to have a PrepareToEat method. Each derived type of Fruit will implement PrepareToEat differently (bananas will peel, pineapples will slice). Then, eat can become an operation that can be done on a Fruit ...

public void Eat( Fruit f )
{ 
  f.PrepareToEat(); // implements the proper preparation peel, slice, etc.
  f.Eat();
}

Per comment: I see. It's an "incorrectly designed object hierarchy that violates the the Liskov Substitution Principle." Meyers writes extensively on this in Effective C++ and More Effective C++.

Answer 8

It's just method overload replacement for dynamic OOP languages.

Nothing special. Quite common and fairly legal.

Look at any serious JavaScript code to see it in action (f.e. ExtJS).

Answer 9

It's an ad-hoc implementation of the Lisp TYPECASE macro.

Of course, in Lisp it's much less verbose, and there's no casting (everything is basically generic by default), so while it's relatively uncommon, it doesn't have quite the stigma it does in statically-typed languages:

(typecase fruit
  (banana (peel fruit))
  (pineapple (slice fruit))
  (strawberry ()))
(eat fruit)