0

I know it's not common to use goto statement. However, I don't know if it's necessary to replace it for something else here.

This pseudocode is kind of a state machine were depending on some conditions I need to be able to "jump" to a certain point. This code is called whenever a bit is received via serial communication.

switch (Actual_state)
{               

    case "FirstState":

        if(restriction)goto case "FourthState"; 

        //state code
        Actual_state = Next_state;
        break;

    case "SecondState":
        //state code
        Actual_state = Next_state;             
        break;

    case "ThirdState":
        //state code
        Actual_state = Next_state;
        break;

    case "FourthState":
        //state code
        Actual_state = Next_state;
        break;

    case "FifthState":
        //state code
        goto case "SixthState"; //if the statement code is complete I need it to go directly to next state, else it would overwrite some info and I don't want more variables. make any suggestion you want.
        break;

    case "SixthState":
        //state code
        Actual_state = Next_state;            
        break;

    case "LastState":
        Actual_state = "FirtState";
        break;
}

Everything is working fine, but I want some opinions on this workflow. If it's garbage feel free to say so.

Pabloczul
  • 21
  • 6
  • Did you write it or are you new at maintaining this piece of code? I'm only asking because with a running system, you could simply leave it as this. If you have freedom to change than that's a complete other matter :) – Icepickle May 03 '19 at 23:11
  • That is invalid and incomplete code, but I think I see what you're asking. It looks like those `goto` calls could be replaced with calls to methods instead. How do you expect any of the code after a `goto` inside a specific `case` block to ever execute, though? Perhaps if you posted some kind of valid example it would help (though the question is still off topic). Right now it looks completely hypothetical and not useful at all. – Rufus L May 03 '19 at 23:19
  • 2
    As far as I know, a `switch` fallthrough is pretty much the only place where a `goto` would be considered acceptable. Personally, I think there's nothing wrong with it. For more info, check out https://stackoverflow.com/a/174223/8829944 – hensing1 May 03 '19 at 23:34
  • I assume that you meant `goto case "FourthState"` and not `goto FourthState`. – Eric Lippert May 04 '19 at 00:14
  • @henry.dv there's also the nested foreach! – Jonesopolis May 04 '19 at 00:45
  • @henry.dv that's exactly what I was asking for! thanks – Pabloczul May 08 '19 at 13:41
  • @Icepickle, I wrote the code based on earlier work from a partner. But I'll refactor it as suggested in one answer it's not the proper way to implement a state machine. – Pabloczul May 08 '19 at 13:49

1 Answers1

4

This code cries out to be refactored.

State machines are a common pattern; if you want to use that pattern then I suggest that you actually implement a state machine type. What is the nature of a state machine?

  • It maintains a state, starting with a start state
  • It takes an input
  • Based on the input and state, it produces an output (or an effect, but preferably not both), and enters a new state

interface IStateMachine<TIn, TOut, TState>
{
  TOut Next(TIn input);
  TState State { get; }
  bool IsHalted { get; }
}

Now can you implement a base class abstract class StateMachine<TIn, TOut, TState> that implements the logic of a state machine by implementing this interface? Pay particular attention to how you represent the transition function.

Now can you implement your logic as derived type from your base type?

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067