Why most uses of State pattern are code-smell

refactoringI am now reading “Refactoring: Improving the design of existing code“. It’s quite a nice reading and, unlike my previous tries to read this book, I tend to say “of course, doh!” instead of my usual “aha!”. But that doesn’t mean that I fully agree with everything in this book.

One thing in particular troubles me, and it’s actually a bad habit I seen with most of the managed languages programmers. They tend not to care about how many things they allocate since, well, they feel like they live in a world where memory is for free. And it’s not.

The culprit in this case is the State pattern. It’s one of the GoF patterns, so it must be good, isn’t it? It consists of creating for each possible state of an attribute a different class. Martin Fowler’s example refers to price categories for a movie, and he replaces a switch with the creation of some classes derived from a base class called Price that has one abstract method ‘getCharge’. Now all he has to do is set the price code with a method like this:

[code language=”cpp”]public void setPriceCode (int arg)
{
switch (arg) {
case REGULAR: _price = new RegularPrice(); break;
case NEW_RELEASE: _price = new NewReleasePrice(); break;

}
}[/code]

How does a Price class look like? It’s quite simple as well:

[code language=”cpp”]class RegularPrice…
public double getCharge (int rentalDays)
{
return rentalDays * 1.5;
}[/code]

So why do we need an object of type ‘RegularPrice’? We need them because we want to calculate the charge, but we notice instantly that the getCharge call uses no members of the RegularPrice class. If this is so we could make it static; but OO programmers don’t really like that because they can’t really inherit when they have static methods (you can’t have static virtual). So from their point of view everything is fine and dandy, and OOP. They feel like the fifth guy in the GoF.

So where’s the problem? The problem is in the ‘new’ statement. You allocate space for an object that is in fact a marker, only used to call an implementation. An object doesn’t come for free: in managed language any object already has a vtable and space for synchronization primitives that can be used on it; in this case an extra method will exist in the vtable. Even in C++, a Zero-Memory-Object would cost, since the object must have a designated address that is unique, therefore at least the minimum allocation size (4 or 8 bytes) will be allocated for the object. And please remember that memory allocation is not an O(1) operation.

Is it such a big waste?

As a programmer that lived most of his programming life among memory deficient and embedded devices, I can tell you it is. It is not only because of the waste itself (although you’ll run with less bytes with each instance). It is bad because of a wasteful manner of thinking that is behind this pattern. I am pretty sure that any pragmatic programmer will understand why this is so.

So what can we do? Should we bypass the State pattern altogether? There is a good reason behind using the State pattern in this case: we don’t really want to do a switch everytime we invoke this call. So we might want to use the State pattern in a modified form: we create an instance for each type (and OOP fanatics will use singletons) and we’ll const-reference the objects instead of instantiating new ones everytime we need a RegularPrice, for example. Is that ok?

Still not ok, from my point of view. While OOP programmers are almost happy (they still mutter about my comments on singletons), experienced programmers recognize the danger of having an explosion of classes in your current folder. And they will feel they have to create a new namespace especially for all these micro-classes (and any coding standard will let you know that you want to have one-class-one-file). It is messy.

How would I implement this? Use lambdas instead. The first question that pops into a sane mind is ‘why do I need an object for this?’. You don’t, only Java allocators run in negative time and free memory the more they allocate*. So a C# programmer (I use C# because I find it quite expressive and flexible) would write:

[code language=”cpp”]readonly Dictionary<MovieType, Func<int, double>> PriceCalculator = new Dictionary<MovieType, Func<int, double>>
{
{MovieType.REGULAR, (days) => 1.5 * days},
{MovieType.NEW_RELEASE, (days) => 3 * days},
};

priceCalculator = PriceCalculator[movieType];

return priceCalculator (rentalDays);
[/code]

Little memory has been harmed during the making of this production: the Dictionary is there for code clarity. If you don’t like it, use a switch statement to assign the lambda functions to the priceCalculator and even the Dictionary memory will be spared. I’ll leave the example this way to focus on code clarity over pure performance.

Will the priceCalculator invocation be efficient enough? Yes: we don’t use local context, and the call will be translated in a simple function call. As fast as it gets.

So if you really want to use the State pattern, think of the consequences. I find that my approach improves readability and makes future extensions easy. What do you think?

[footnote mode=”rant”]

*Java programmers are so proud of this language that they assign it never-seen-before-qualities. Java is the only language that runs faster than native code all the time and for Java memory hogging is acceptable when you compare it with the OOP purity that it brings. You’ll find benchmarks in which Java beats hands down any programming language ever conceived. Java programmers tend to be funny that way.

But a lot of times, good programmers do Java. Like any good programmer, they are not language tied, and can easily switch to C#, C++, C, PHP or whatever the project needs, because good programmers transcend programming languages.

One more thing: I’m not suggesting that Martin Fowler is wrong in his refactoring approach. His intention is good, and this is just an unfortunate oversight. It’s not important and it doesn’t really affect the validity of the Refactoring book contents. But this issue is a sign that everything we read we have to take with a grain of salt, because even the good ones make mistakes. Sadly, I see a huge trend of ‘design patterns are kings’ which generates a ton of developers that read GoF and tend to use it as a weapon in the service of evil. I, for one, hated GoF when I wasn’t mature enough to understand it, and ignored it. When I matured a bit as a programmer, reading GoF was a “Yup, that’s how I’d do it too” experience instead of “wow!”.

[/footnote]

Comments

Why most uses of State pattern are code-smell — 1 Comment

  1. I don’t know (yet) enough Java to be sure about it, but wouldn’t it be a better alternative (and potentially a more performant one) to make setPriceCode take an enum that contains the implementation of getCharge as a method? (like this: http://stackoverflow.com/a/18883775/2161939) Btw, enums are IMO one of the few features where Java wins over C# hands down in terms of expressiveness.

    Also, I really agree with you about the whole managed-languages-are-cool-so-let’s-create-objects mindset that is so spread not only on the Java camp, but also in D.