Request for Feedback (State Design Pattern)

Hello,

About the (language) style: it is correct enough, as informal as my own writing style - so you guess it: I find it cool :)

About the subject: you should give an explanation of the state pattern before jumping to the implementation. Iy you do so then you create a road for your readers: a state pattern is this, here is one, here is how it is used, here are the fine tricks to correctly use it, and now you must go to bed.

About the implementation:

• the transitionOutOf(oldstate) and transitionInto(newstate) are not a good idea. First, they are redundant: it is pretty obvious that oldstate->transitionInto(newstate) and newstate->transitionOutOf(oldstate) are semantically equivalent. Second, if you require some particular processing to go from a particular step A to a particular step B then you should not do it in neither a method of A nor a method of B. Since you are palying with states, you can create another state (call it AB) to handle this particular processing. If I don't convince you then try to image that you may have particular processing to go from A to B, C, D, E, F, G, H, I, J, K, L, and so on. You'll be force to write ugly code in A is you want to handle everything in transitionOutOf().
  
  
• I do not consider the App::queueStateTransition() to be a good idea too (there is a I in front of this sentence. Therefore it is an opinion :)). It requires that any state knows what a App is. This fact lowers the level of reusability of your state code. You can handle this in another way: for example, the State::update() function can return something which will enable the App to choose which is the next step. Erich Gamma proposes two options: in the first one it is up to the state to return the pointer to the next state. This is simple to implement, efficient, explicit (I really like explicit code), easy to modify and to extend. The bad news is that it introduces dependencies in the state subclasses. The second solution is nearly yours: it is a table driven implementation. Each state transition is stored into a table and the next state is choosen in the table using the current state and the last input. This is somewhat harder to read (the transitions are less explicit) and difficult to extend - due to the nature of the table.
  Of course, it is up to you to choose the way you want to implement it. As you see, all the possibilities has some drawbacks.
  
  
• public virtual functions should be avoided when possible. For more information read Herb Sutter's "Virtuality" article
  
  
• It seems that the explaination focuses more on the code than on the state design patter itself. Maybe you could add more explaination about why you are doing this, why are you using that and so on.
  

Voilà. I don't think of anything more for the moment. Anyway, good job: the global explaination is good enough to be understandable by someone who already knows C++ and OO design.

Regards,

Emmanuel,

Thanks very much for your comments, I really appreciate them. I have responses:

Quote:

Original post by Emmanuel Deloget the transitionOutOf(oldstate) and transitionInto(newstate) are not a good idea. First, they are redundant: it is pretty obvious that oldstate->transitionInto(newstate) and newstate->transitionOutOf(oldstate) are semantically equivalent.

This isn't true, but it looks like the intent of these functions are not clear. They are called by the App when a state transition occurs. transitionInto() is called to prepare a state object for its processing while transitionOutOf() is called to clean up the state object before it is swapped out. Think of them as onEntry() and onExit(). (In fact, those are much better names for these functions!)

As an example, let's say you have two states: MainMenu and LoadGame. When the application is in the Main Menu state and the user clicks the "Load Game" button the application transitions out of the Main Menu and transitions into Load Game by doing calling MainMenu::transitionOutOf() and then LoadGame::transitionInto(). The pseudocode for Main Menu's transitionOutOf() might be:

void MainMenu::transitionOutOf(State* toState){   Stop Playing Main Menu music   Fade Out Main Menu graphics   Release Main Menu graphic resources}

while the code for Load Game's transitionInto() might be:

void LoadGame::transitionInto(State* toState){   Load graphic resources for "LoadGame" state   Scan disk for all saved games   Populate a list box with all saved games   Fade in LoadScreen Graphics/UI   Start Playing LoadGame music}

Hopefully the intent of these functions are now clear. Let me know if you still find they don't "feel" right.

Quote:

Original post by Emmanuel Deloget Second, if you require some particular processing to go from a particular step A to a particular step B then you should not do it in neither a method of A nor a method of B. Since you are palying with states, you can create another state (call it AB) to handle this particular processing.

This seems like an overkill of states to me. Actually, in all my current cases in the game I'm working on, the arguments of these functions are not used - the entry/exit functions for a particular state seem to be independent of which state it's going to or coming from, but I have put there in case there are some special situations/transitions that require knowledge of the "from" or "to" states. A simple example might be if you want the music to continue to play during one transition and not another...

Quote:

Original post by Emmanuel Deloget I do not consider the App::queueStateTransition() to be a good idea ... It requires that any state knows what a App is. This fact lowers the level of reusability of your state code.

I agree with this and in a longer article I would talk more about the proxy class (i.e. the "State Manager" that I mention) but for simplicity's sake I have made all classes aware of each other to keep the number of source files/modules down in the example.

Quote:

Original post by Emmanuel Deloget public virtual functions should be avoided when possible. For more information read Herb Sutter's "Virtuality" article

Thanks, I intend to read this article and try to understand why this is a Bad Thing (tm).

Again, thanks for your time!

Regards,
Jeff

I don’t agree with it as a rule of thumb; protected virtual's implies a strategy pattern exposed through the public member functions, such as the case with the iostream stuff. If a strategy is optimal, then that’s a C++’ized implementation, but if a strategy isn’t called for it’s not the right thing to do.

I added a Part Three some time ago that deals with mouse events and how to handle them.