Martin Fowler's Refactoring book: switch statement is using other object's data, why is that bad ? A deeper explanation is sought for.

Question

In the code sample below the Rental object is using Movie's fields to do a switch statement. Martin says : this is a bad idea but he does not give any deeper explanation as to why ?

Of course, you can say that this means a lot of coupling between Movie and Rental, or that Movie is badly encapsulated but those are just (almost meaningless) words, again, they don't explain the why? They don't go to the root of the question, so to say. They don't provide much understanding.

What I would like to know if there is a deeper, more general design principle why having a switch on Rental is a bad idea ?

In other words, how does Martin knows that the switch statement is on the wrong object ? Where does he get this insight from ? What is his thought process ?

Answer 1

What happens when Movie has a new type added to it like Movie.DISCOUNT?

Of course, you can say that this means a lot of coupling between Movie and Rental, or that Movie is badly encapsulated but those are just (almost meaningless) words, again,

They're deeply meaningful words. They mean that when you change Movie, you now get to change Rental too since they're tightly coupled. Worse yet, in a language like Java there's nothing to tell you that this switch statement needs to be modified (though most IDEs do, to be fair). So if you don't modify the switch statement, you now have a bug that's giving people free movie rentals.

The primary focus of program design (and by extension refactoring) is optimizing code around the question "what happens when this changes?". That's the thought process that will help you understand many of the motivations in the book.

Answer 2

There are two issues:

1. Since the priceCode is part of the Movie class, the switch should likely be located within the Movie class. That localizes the work in the switch to the class which define what a priceCode is. If the Movie class were to decide to return different codes, then it is in the best position to modify the switch.

2. The switch is probably not the only thing in the Movie class which depends on the priceCode. This would suggest creating derived classes of the Movie class, one per priceCode. Each would have its own overridden version of getCharge. This overridden method would not only have the separate algorithms which are currently in case statements, it could also depend on data that is specific to the individual derived classes.

Answer 3

I can only agree with that if PriceCode is another object. In this case, it would be more appropriate to make the switch like PriceCode.REGULAR and not Movie.REGULAR for example. So Martin would be talking about Movie having constants defined on PriceCode, which looks like bad design.

Or maybe he's idea is to think on classes with very low coupling, for example, you want Rental to work with another class compatible with Movie but that is something else (for example, you want to rent a CD instead of a Movie).

Probably he's talking about the second option.

Answer 4

The core concept of OO is to encapsulate the operations close to the data. Fowler's idea is that movie should decide itself how much it is worth (may be through inheritance into multiple movies?).

I do not completely concur with Fowler's idea:

1. There are other ways to deal with this. What if 'rental policy' is an object in itself? Movie is supposed to represent the purest data about the movie, and not how a rental company considers a movie to be a rentable object.
2. This does not work in enterprise applications: data gets serialized, transformed, dissected, split, joined... ensuring Fowler's idea would be hard.
3. What is the unit of encapsulation? Is it better to consider renting as a composite? This composite owns these data elements of movie and rental policies, and offers a service that would be the single point of decision making.

Many a times, Fowler's ideas are usually best suited for teaching how to build small applications. These principles should in turn help build clear thoughts of building larger applications that contain composites.

Answer 5

Why is relying upon the properties (or fields) of a another class to make a calculation a bad idea? The answer is inherent in the title of the book: refactoring. If changes are made to that class, the scope and meaning of the data can change. If a new movie type/price code is created, this code also needs to be adjusted.

Which points out the more precise problem -- relying upon a enum/type in another class. If Rental used a Renter class that had an age, and offered discounts based upon age, that wouldn't be such a problem. Age is unlikely to change. If on the other hand, our hypothetical Renter class had a Renter.Type (corporate, family, individual, minor, senior citizen), that would be much more likely to get modified, thus breaking Rental.