The problem arises when the user triggers a state change from another thread (Swing). Lets say the main thread is in the execute method of IDLE when it gets interrupted by the GUI thread. The user chose to start a new game, so setState(State.GAME) is called asynchronously. Now, the main thread continues with the execution of IDLE and returns State.IDLE. State.GAME is overridden and the state machine is back in an IDLE state. The users action is ignored.

0

Share this post

Link to post

Share on other sites

Basically the problem comes down to making sure setState(state.execute(this)) doesn't get interrupted. If a state change is requested by the user, it needs to happen before or after setState(state.execute(this)). One naive attempt for a solution would be to make StateMachine.setSate synchronized. That doesn't work though, since error occurs when the main thread is interrupted during State.execute.

0

Share this post

Link to post

Share on other sites

Original post by kloffyBasically the problem comes down to making sure setState(state.execute(this)) doesn't get interrupted. If a state change is requested by the user, it needs to happen before or after setState(state.execute(this)). One naive attempt for a solution would be to make StateMachine.setSate synchronized. That doesn't work though, since error occurs when the main thread is interrupted during State.execute.

Well, there are actually many more problems, mostly that the design doesn't make any sense.

Why is StateMachine a Runnable? It doesn't do anything by itself.

Why is the enum designed the way it is, when it performs exactly the same role as the class it's contained in?

Why does it accet StateMachine when it's called only with this?

Why aren't you propagating the changes between states via observer pattern, or using vetoable listeners?

Yes, the proper solution for synchronization is to make things synchronized on a common lock. If you have any other issues with this, or if this isn't adequate, then you'll need ensure transactional safety, or change the way it's used.

Of course, your original code might not be what you really have, so a different design would be better suited.

The core of your design problem comes from making StateMachine constantly running (while (...)), whereas StateMachine, by very definition is event driven (it's idle, until something happens, upon which it responds).

Alternatively, you could make StateMachine passive, and check and handle its state in a single place, thereby bypassing the threading issues altogether.

0

Share this post

Link to post

Share on other sites

Thanks for your suggestions. Obviously the code I provided was just an example, the real code is different. In my real application every state performs some calculation during State.execute, except for the IDLE state. The reason State.execute gets StateMachine as a parameter is to give the states a way to manipulate StateMachine data. Making the StateMachine passive could be an option, I'll have a look into that and see how it plays out.

0

Share this post

Link to post

Share on other sites

I agree with the recommendations Antheus makes, but they stop at taming state updates. There is a more fundamental problem: accessing the state from outside (public setState() method) is utter nonsense, because the state machine is the sole responsible of its state and must change it of its own accord only (in this case in the run() method, which is a bad idea for different reasons).

Nothing outside the class in question should know there is a state machine, not to mention what states it has; any meaningful class has methods that might represent commands or even imply state changes (e.g. pauseGame() or resumeGame() for a main loop manager), but the fact that one of the things an object does when they are called is switching a state variable (e.g. between GAME and PAUSE) is a private and inconsequential implementation detail.

0

Share this post

Link to post

Share on other sites

I cannot quite imagine how to implement this. I would appreciate I you could post a simple example (with a main loop and some state changes). It doesn't need to do anything, just to give me an idea how it would look in code. Thanks in advance!

Edit:Here's a design that's pretty similar to what I tried to do. Maybe his example is better.