Is this a bad use of GoTo?

Started by
10 comments, last by Bregma 12 years, 7 months ago

Is this a good use of GoTo? This code is supposed to mimic a FSM. Personally, I see nothing wrong with it. Given the function is pretty short. But I wonder if this approach should be avoided: because of a better solution, or just because GoTo is the spawn of evil itself.


As many have noted, goto by itself is not the spawn of evil. There was a recent discussion which included the pdf and the context of the original argument about why gotos should be avoided.

Using goto when other control statements are a better fit is the evil action. Goto is almost never the right answer. There is almost always a better structured answer available. There are very rare situations (generally during error handling) where the unconditional goto jump is the right tool. In almost every other situation goto is the wrong answer since a structured control statement is more clear.


In your specific example you have just three states. The first one drops through, the second redirects to one of two other states, the third resets.

You can see that your example won't scale well. It already struggles for clarity with those three states. Where does each state go, and under what conditions? If you add a state between 0 and 1, what states will the new state transition to? Should the new state fall through to the next state uninterrupted, or possibly route elsewhere? What happens when you want to insert a state in the middle of states 1 and 2?


With three states it is already hard to follow. At double that size it would become a maintenance nightmare. Heaven help you if the monster grew to hundreds of states.


That is why goto is a poor fit.




As others have noted, an FSM is best expressed by a switch statement or other lookup table directly. In vb.net that is a select/case tree.

My preference for a VB example would be along these lines:
[source lang='vb']
Enum FSM_States
State_crash = 0
State_Start
State_Idle
State_Heating
State_Cooling
...
State_End
End Enum


state = State_Start
While state <> State_End AND state <> State_crash
Select Case state
Case State_Start
state = DoStartState()
Case State_Idle
state = DoIdleState()
Case State_Heating
state = DoHeatingState()
...
Case Else
Debug.Assert("Unknown state! Crashing the state machine.")
state = State_crash
End Select
End While
[/source]

This structure very closely follow the details of an FSM. The machine only cares about your current state and the transition that follows on completion. The machine does not care about the internals of any state. Each state must return the transition of the state that follows.

Maintenance and performance wise this structure is excellent for quite a few states. At some point it will eventually be easier to maintain as an array of function pointers, but that would be far beyond a For Beginners post.
Advertisement
This is an excellent use of goto. It's a wonderful example of the sort of use of goto that lead to Dijkstra's famous essay. We used to call this spaghetti code (as in, pray to the FSM that you don't have to maintain that code).

After all, if you can't be a good example, you can still function as a warning.

Stephen M. Webb
Professional Free Software Developer

This topic is closed to new replies.

Advertisement