4

I am developing a bot which I need to start and stop at will.

I have 2 buttons in my form, StartButton and StopButton which change the state.

The Bot has 4 possible states: starting, started, stopping, and stopped. I am now trying to manage the state transitions. So far I have 4 boolean variables, one for each state. But that leads to a lot of repeated code. Is there a better design?

In my BotState.cs class I have this:

public static bool Starting = false;
public static bool Started = false;
public static bool Stopping = false;
public static bool Stopped = false;

public static void SetStartingState()
{
    Starting = true;
    Started = false;
    Stopping = false;
    Stopped = false;

    MainForm.Instance.Invoke(new MethodInvoker(delegate
    {
        MainForm.Instance.StartButton.Enabled = false;
        MainForm.Instance.StopButton.Enabled = false;
    }));
}

public static void SetStartedState()
{
    Starting = false;
    Started = true;
    Stopping = false;
    Stopped = false;

    MainForm.Instance.Invoke(new MethodInvoker(delegate
    {
        MainForm.Instance.StartButton.Enabled = false;
        MainForm.Instance.StopButton.Enabled = true;
    }));
}

public static void SetStoppingState()
{
    Starting = false;
    Started = false;
    Stopping = true;
    Stopped = false;

    MainForm.Instance.Invoke(new MethodInvoker(delegate
    {
        MainForm.Instance.StartButton.Enabled = false;
        MainForm.Instance.StopButton.Enabled = false;
    }));
}

public static void SetStoppedState()
{
    Starting = false;
    Started = false;
    Stopping = false;
    Stopped = true;

    MainForm.Instance.Invoke(new MethodInvoker(delegate
    {
        MainForm.Instance.StartButton.Enabled = true;
        MainForm.Instance.StopButton.Enabled = false;
    }));
}

So as you can see there is a lot of repetition. I am not sure how to make it better.

I thought of using an Enum for the possible states but im not sure if it would eliminate repetition on changing the Enabled state in the MainForm controls.

  • (please ignore the previous comment, I now see that this is a legitimate design-level question, just one with a large code example.) – amon Oct 29 '17 at 17:53
  • thanks for the heads up, thats actually wrong in this post. I will edit it. Its not like that, i dont know why i posted it this way... – Joao Vitor Oct 29 '17 at 22:37
  • Not sure if you can do something similar in C#, but in Java we are able to specify fields and constructors for each enum instance, so you could also have a map of allowed state transitions in each state enumeration, minimising the code needed to validate and perform any given transition to a single, simple method or function. – Maybe_Factor Oct 31 '17 at 0:15
  • could you point me to an example in java? – Joao Vitor Oct 31 '17 at 14:22
5

Definitively, an enum could be used for representing the state in a single variable and avoid some repetitions.

Take the following definition:

public enum State {Starting = 0, Started=1, Stopping=4, Stopped=6}; 

You could then have a single SetState() function:

public static void SetState(State s)
{
    current = s; 

    MainForm.Instance.Invoke(new MethodInvoker(delegate
    {
    MainForm.Instance.StartButton.Enabled = (2 & (int)s) !=0 ;
    MainForm.Instance.StopButton.Enabled = (1 & (int)s) != 0;
    }));
}

Here an online demo showing what this provides as result for each state.

Ok, I confess: I tricked a little bit with the values of each enum element, so to allow playing with binary operations. But this was only for a compact proof of concept. In real life you could use some mapping function/table which would provide the correct vales to be used for each variable and for each state.

Another thing that you could consider, could be to make this more evolutive by implementing the State pattern. You could combine this pattern together with the template pattern, so to have a template method that would do all the repetitive stuff when changing the state, and that could invoke state specific methods, for the non-repetitive things.

  • In real life I would do something a lot cleaner :D The start button only needs to be enabled if the state is Stopped, and the stop button when the state is Started. No need to mess around with bits. – Peter M Oct 29 '17 at 20:10
  • @PeterM as I confessed (second paragraph from the bottom), I would not either trick with the bits in real life. Time for my favorite quote from B.W.Kernighan : "debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?" – Christophe Oct 29 '17 at 20:19
  • I know the quote, but also personally believe that it is the best interests of everyone to provide the simplest example possible in an answer. The use of bit mappings isn't going to help the OP (or anyone else) who doesn't realize that you use of them is facetious. – Peter M Oct 29 '17 at 20:25
  • Thanks for your answer Christophe, so lets say i wanna add more states like Loading(All buttons enabled = false) and Loaded (just star button enabled) i think that also could work.. – Joao Vitor Oct 29 '17 at 20:42
  • 2
    Could you share what you have in mind @PeterM? – Joao Vitor Oct 29 '17 at 20:43
2

This is untested code. (And I am also unfamiliar with Winforms. When I do desktops apps I use an C#/WPF/MVVM approach - so I would do this totally differently, but also recommend that you explore that way of doing things as well)

From your original post and comments, the Start button is only enabled when the Bot state is Stopped or Loaded, and the Stop button is only enabled when the Bot state is Started. For all other states the buttons are disabled. So my approach would be to define an enum and use the current enum value to set or clear the control enables.

public enum BotState 
{ 
  Unknown, 
  Stopped, 
  Starting, 
  Started, 
  Stopping, 
  Loading, 
  Loaded 
}

public BotState BotState = BotState.Unknown;


public void SetState(BotState newState)
{
  BotState = newState;
  MainForm.Instance.Invoke(new MethodInvoker(delegate
  {
     MainForm.Instance.StartButton.Enabled = (BotState == BotState.Stopped || BotState == BotState.Loaded);
     MainForm.Instance.StopButton.Enabled = (BotState == BotState.Started);
  }));
}
  • Thanks! From what i see its kinda similar from Christopher's way but more "english"ish, which is always good. It's good to note that all States ending with "ing" (meaning something that cannot be interruped is in progress so the user can't stop or change the course of it) should make the buttons disabled. – Joao Vitor Oct 29 '17 at 21:30
  • @JoaoVitor Fundamentally it is the same approach. I just differ on the details. – Peter M Oct 29 '17 at 21:43
  • This is a nice one as well :-) However, exactly as my binary version, it will be very intolerant to evolution. Suppose that some time you'd like to add intermediary states between started and stopped. You'd need to revise all your enable logic, and the one for the start button would become very long. This is suboptimal in view of the open/close principle. This is why it would be more maintainable to introduce some mapping function, or a mapping table that you would fill with just the desired value for each state. – Christophe Oct 30 '17 at 0:32
0

To speak to greater design principles, there is essential state and derived state. Regarding derived state, we can have (derived) state computed (from essential state) on demand, and (derived) state stored (aka cached) from an earlier computation.

Ideally, essential state is compact and also changed atomically. If we were working in database field we'd say normalized for compact, and in other fields, we'd say DRY (don't repeat yourself).

Both ideals of compact and atomic suggest that using a single variable for holding the current state is better than using multiple variables.

If I read between the lines about the intent of the states in your question, in some sense, the situation calls for 2-bits of state (e.g. one enum of four values), but you're using 4 bits (four booleans). So, there is an expansion over the essential state required (i.e. suggesting that there is some derived state). Further, this derived state is being stored, meaning is being cached rather than computed on demand.

On such a simple code base these are probably not significant concerns.

However, if this were a larger code base, we'd say that caches can suffer from being stale (showing old values). There could also be potential to see intermediate values (showing an incorrect value during a non-atomic transition between states) — generally speaking that would require threading, though perhaps still worth mentioning.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.