Avoiding code duplication in state pattern

[MarkCG]

Invariably, in most large projects I do, I end up needing to do one thing which I have not found a clean way to do yet. And that is handle messages between components differently based on state of the component / actor.

Simple example of what I mean-- a machine that can be in an idle or running state. If machine is off/idle, a shutdown message to the actor or component representing the machine does nothing- the message is read but nothing happens, it is already off. If running, it goes through shutdown sequence, actually controlling physical hardware. Likewise, if running, a "Run message" has no effect.

In the past, pre-oop, I handled by separating the messaging from the states. Essentially nesting your standard enum+variant dequeue + case structure INSIDE each case of the state machine. So inside the while loop, a case structure for each each state, and a case structure to handle messages inside each case of the state machine. This worked fine and was easy to debug, but it was cumbersome and one had to be very diligent about handling every message in every state correctly.

Now I see I can apply the State Pattern to solve the above problem-- abstract methods in a parent class, with child "state" classes that have concrete implementations of those methods for each message where needed.

However, there something about this that doesn't quite sit right with me. Let's see if I can explain it via some pseudo-code

ParentState

----- method_1() {}; //abstract method

----- method_2() {}; //abstract method

ConcreteState_A :: extends ParentState

-----method_1() {do stuff;} // only method 1 is implemented in State A. Method 2 should do nothing.

ConcreteState_B :: extends ParentState

-----method_2() {do other stuff;}// only method 2 is implemented in State B. Method 1 should do nothing.

ConcreteState_C :: extends ParentState

----method_1() {do stuff;}//same implementation as in ConcreteState_A

----method_2() {do other stuff;}//same implementation as in ConcreteState_B

ConcreteState_A does only method 1, ConcreteState_B does only method 2, ConcreteState_C can do either, and the implementation is the exact same as in A and B respectively.

Clearly I can move the concrete implementations of method_1 and method_2 up to the parent class. Sure. But in order to make ConcreteState_B do nothing when method_1 is called on it, I would have to override method_1 with an empty method!!! Otherwise the parent method_1 will get executed (dynamic dispatch), and something will happen when I don't want it to. Likewise for ConcreteState_A, I will have to override method_2() with and empty method. This seems like a backwards way of doing it.

I could leave the ParentState methods abstract, and I could move all the method_1 functionality into a function and use method_1 as a wrapper, but I would have to still create overrides for each concrete state that needed to implement method_1, duplicating code.

I Imagine that I had 20 classes, each needing to be able to implement / not implement a large combination of methods, things would get hairy with the above solutions.

What is it that I am looking for here? Either of the above methods seems kludgey. Is there a way to explicitly say to a child class--- "I know the parent has concrete method implemented, but YOU are not allowed to execute it!" (do as I say, not as I do )

Are "Interfaces" what I am looking for??

Edited December 4, 2012 by MarkCG

[PaulL]

Mark,

I think this is a well-posed question!

I have some suggestions, and I will try to put them in some coherent order:

1) I recommend keeping the top-level state methods abstract (thus maintaining an interface to the invoking context). (If there is a need to implement concrete methods shared by all states, consider creating a concrete class directly beneath the interface.)

2) I also recommend putting all the behaviors within the state methods (except for transition logic, which we, at least, put in the state methods directly) into another class (Context or Model, preferably via an IContext or IModel interface). Prepare IModel (I will describe this option) methods for the largest reusable units the state methods will invoke. (I don't think it is bad to reuse a particular IModel method in different state methods, but if two state methods call the exact same sequence of IModel methods, and hence do exactly the same thing, which is what you described, then I think it is worth considering refactoring.)

3) Most importantly, consider carefully the hierarchy of states. The hierarchy certainly does not need to be flat! Maybe you can end up describing (and hence grouping) states in a different way. In the example above, what is the relationship of ConcreteState_C to the other states? (From what you describe it is possible that the system actually has two orthogonal state machines, with C being a sort of combination of A and B, although there is not enough information in the description to determine that.)

4) There might be some situations where repeating implementations is appropriate, for example, if the implementations could vary independently in the future (which is likely the case if the states logically don't have any sort of inheritance relationship between them).

Paul

[mike_spacex]

Paul is right, you need more information to know if there's a better way to structure your hierarchy. From what you told us, any of the following diagrams may be valid:

A Venn Diagram is not a great tool for modeling behavior, but hopefully you see the point: You need to take into consideration all the behavior (and possible future behavior) you may want to incorporate into each state before settling on a design.

One of the great things about the state pattern though is that it's very forgiving. If you do as Paul suggests and separate your implementation from your state methods, then molding it into a new hierarchy or adding or removing states & methods is fairly trivial.