Is branching an OOP code-smell?

by

A code smell is a hint that something has gone wrong somewhere in your code. Use the smell to track down the problem

Kent Beck via http://c2.com/cgi/wiki?CodeSmell

I’ve developed a new code-smell sensitivity to “if.” In fact, branching in general triggers an alarm as I read through code (especially if I just typed it). 

Can something so fundamental to programming be a code smell? That gets me thinking that maybe it isn’t so fundamental to OOP.  In high academia, I was told that “structured programming” was the greatest formalism since the slicing of bread. There are 3 required pillars to structured programming:

  1. Sequence – A set of statements are executed in a certain order.
  2. Selection – Statements are executed conditionally.
  3. Iteration – A set of statements can be executed multiple times.

In light of declarative programming and OOP, all of these may be a code smell. For now, I’ll just tackle ‘selection’ and come back to the others as time permits.

Lest I wax too abstract, let’s start with some code (in pidgin java):

switch (pizza.crust) {
   case Pizza.CRUST_THICK:
      doughInOunces = 16 ;
      bakeTimeInMinutes = 13;
      break;
   case Pizza.CRUST_THIN:
      doughInOunces = 12;
      bakeTimeInMinutes = 8;
      break;
   case Pizza.CRUST_PAN;
   default:
      doughInOunces = 19;
      bakeTimeInMinutes = 18;
}

Here we have a selection, a choice if you will.  How much dough do we need and how long do we bake it? Well that depends on the thickness of the crust. Another way of saying that is that it depends on the type of pizza. That’s our first hint. First a detour through some smarter minds than mine.

I’m a big fan of the “Tell, don’t ask” principle and its close relative, “The Hollywood Principle.” Essentially, you don’t ask an object about its state and then act based on that state; you tell the object what you want it to do. In my example, the client code asks the pizza object about its state and makes a decision based on that state. That decision probably belongs near the state itself.

Here’s my proposed alternative (again in pidgin java):

class ThinCrust extends Pizza.Crust {
   int getDoughInOunces() { return 12; }
   int getBakeTimeInMinutes() { return 8; }
}
class ThickCrust extends Pizza.Crust {
   int getDoughInOunces() {return 16; }
   int getBakeTimeInMinutes() { return 13; }
}
class PanCrust extends Pizza.Crust {
   int getDoughInOunces() { return 19; }
   int getBakeTimeInMinutes() { return 18; }
}

And here’s the modified client code:

doughInOunces = pizza.crust.getDoughInOunces(); 
bakeTimeInMinutes = pizza.crust.getBakeTimeInMinutes(); 

Recall the ‘type’ hint. If you have a choice based on some type of something and you’re using a language with a type system (if not, I’d really like to hear about it), use that! In my modifications I created an inner type ‘Crust’ for the ‘Pizza’ type. Now the ‘selections’ are tucked into the types.

In the client code, I just tell the object of a certain (and probably unknown-at-compile-time) type to give me the data I need.

Okay, okay, I’m actually asking for data again. With more refactoring, my client code should actually just look like this:

pizza.bake()

The behavior that uses the values for baking time and amount of dough would still be needed, but only inside the pizza object. The pizza object itself would still want to get these values from its crust inner type, perhaps composed through an inversion of control (IoC) container like Spring.

Is there a case where one needs to perform selection instead of deferring the decision to a type? That is, what are the exceptions to my argument above?

Advertisements

5 Responses to “Is branching an OOP code-smell?”

  1. jim siegienski Says:

    If I may be so bold:

    Every time you write a conditional an abstraction loses it’s wings.

  2. ty.delong Says:

    What would you do in a factory pattern when deciding what kind of object to instantiate? For example, if this piece of data is present, return an instance of Object A, else return an instance of Object B?

  3. jonwolski Says:

    Ha Ha! Jim, that’s a better way of stating what took me an entire blog post.

  4. jonwolski Says:

    That’s a good question, Ty. I think in that case, “if” is certainly warranted.

  5. jim siegienski Says:

    Ty,

    Sometimes there are other ways to make that determination. To fully move away from ‘if’ statements to determine behavior you sometimes have to unwind a long way. (this can become a significant refactoring effort and at that point should be managed as such).

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s


%d bloggers like this: