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

1 Answer

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

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.