Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have the task to implement the state pattern in one of my classes. The assignment referenced a suggested implementation, and upon reviewing that implementation I was confused since I believed it to be nonsensical for most applications and frankly simply bad from a design point of view, since it completely dismisses some very basic design principles of software code.

This is the class diagram of the suggested implementation:

enter image description here

I wonder: What is the use of this implementation? It (imo) clearly violates the two main design principles of software engineering: Make things easily maintainable, and easily extensible.

In my eyes, this implementation has zero easy extensibility(you have to change the source of at least two classes, for example, to implement a new state that you forgot about when first implementing it). The coupling between this code and client code seems to be huge. If you remove anything in this pattern, all client code will break. If you want to add anything, you have to change all client code in order to adapt, for example, a new state, because new states can only be entered via the enter() method of the concrete state, or by directing injecting them into the Controller using the changeState() method.

As a bonus, you can only have one garage door in the entire world, since the states are practically singletons. The enter() method in the concrete states was given as this:

public static State enter(Controller c) {
if(m_instance == null) m_instance = new <<concreteStateName>>(c);
return m_instance;
}

Or you can have multiple garage doors, which all open simultaneously if you open one.

Why would you put all methods in the abstract State class? Where's the polymorpy? Why do concrete states have either empty or exception throwing versions of methods that have nothing to do with their state (for example, the resulting interface of Opened will have a "lock()" method, even though you can't directly lock the garage door when it's open. You have to close it first). Why not choose a single method and implement it differently in the concrete States, making the thing easily extensible again?

It is also (again, as far as I can see) not really maintainable without a lot of troubles. What if you, after a couple of months of using the garage, decide that one state is in fact not only obsolete but wrong? You're going to have to take it out, and the according method in State() with it, breaking the entire code base that uses this implementation. This is a garage door with four states. I wouldn't want to work on that thing if it had any more states.

The situation gets more confusing when I look around the internet: The pattern actually seems to be implemented exactly this way a lot of times (for example it is implemented like this way in our national Wikipedia (German)).

The actual question: Am I missing anything or is this simply bad? Since this is so widespread, I'm thinking that I might be missing something obvious here.

share|improve this question
add comment

2 Answers

up vote 3 down vote accepted

an easier way would be to have the state methods return the state after the operation, and combine all methods in the abstract State into a generic procesInput(data,controller): state

this way states don't need to know what controller they are linked to (allowing them to be true singletons)

and now each state then only needs to know what next states are possible according to what input

share|improve this answer
1  
Indeed! That's how I ended up implementing it. The question is more about the possibility of me simply misunderstanding the point of the pattern, or missing something else. –  heishe Apr 27 '12 at 20:56
add comment

Try not to get caught up in the details of the UML. It is extensible if you assume enter(Controller) is some kind of interface implementation, and your concrete methods aren't required to implement all methods of the abstract superclass. The goal of the diagram is to illustrate the essence of the pattern.

The diagram alone fails to convey this, but the payoff is the simple fact that the Controller instance doesn't need to keep track of state to make correct decisions. For example, the controller can call lock() at any time on the abstract m_state object without fear of double-locking the door, or locking the door in an open state, because Closed happens to be the only concrete State object that implements lock() (polymorphy).

This is the reason all the methods defined in the abstract superclass are selectively implemented in the concrete classes. It saves your controller the trouble of writing validation code for invalid state changes like double-locking a door.

The state switching logic is contained in the concrete state implementations. If the current state is a Locked instance and unlock() is called, the Locked state object would call changeState(Closed) on the Controller object (in addition to other Controller methods that might apply, such as a theoretical controller method setIndicatorLight(Color)).

The key here is that the concrete State implementations are designed to interact with each other. Lets say I create another concrete State implementation like Broken. If none of the other states actually set the Broken state on the controller, then the state might as well not exist. You also logically wouldn't create concrete implementations that didn't lead to other states.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.