11

I've been given some Java code to look at, which simulates a car race, of which includes an implementation of a basic state machine. This is not a classical computer science state machine, but merely an object that can have multiple states, and can switch between its states based on a series of calculations.

To describe just the problem, I've got a Car class, with a nested enum class which defines some constants for the state of the Car (such as OFF, IDLE, DRIVE, REVERSE, etc). Within this same Car class I have an update function, which basically consists of a large switch statement which switches on the cars current state, does some calculations and then changes the cars state.

As far as I can see, the Cars state is only used within its own class.

My question is, is this the best way of dealing with the implementation of a state machine of the nature described above? It does sound like the most obvious solution, but in the past I've always heard that "switch statements are bad".

The main problem I can see here is that the switch statement could possibly become very large as we add more states (if deemed necessary) and the code could become unwieldy and hard to maintain.

What would be a better solution to this problem?

  • 3
    Your description doesn't sound like a state machine to me; it merely sounds like a bunch of car objects, each having its own internal state. Consider posting your actual, working code to codereview.stackexchange.com; those folks are very good at providing feedback on working code. – Robert Harvey Sep 1 '16 at 1:32
  • Perhaps "state machine" is a bad choice of words, but yes, bascially we have a bunch of car objects which switch on their own internal state. The system can be described eloquently with a UML state diagram, which is why I titled my post as such. In hindsight, it's not the best way to describe the issue, I'll edit my post. – PythonNewb Sep 1 '16 at 1:34
  • 1
    I still think you should consider posting your code to codereview. – Robert Harvey Sep 1 '16 at 1:38
  • 1
    sounds like a state machine to me. object.state = object.function(object.state); – robert bristow-johnson Sep 1 '16 at 1:52
  • All the answers given so far, including the accepted answer miss the main reason that switch statements are considered bad. They don't allow adherence to the open/closed principle. – Dunk Sep 1 '16 at 16:37
13
  • I turned the Car into a state machine of sorts using State Pattern. Notice no switch or if-then-else statements are used for state selection.

  • In this cases all states are inner classes but it could be implemented otherwise.

  • Each state contains the valid states it can change to.

  • User is prompted for the next state in case more than one is possible, or simply to confirm in case only one is possible.

  • You can compile it and run it to test it.

  • I used a graphic dialog box because it was easier that way to run it interactively in Eclipse.

enter image description here

The UML diagram is taken from here.

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import javax.swing.JOptionPane;

public class Car {

    private State state;
    public static final int ST_OFF=0;
    public static final int ST_IDDLE=1;
    public static final int ST_DRIVE=2;
    public static final int ST_REVERSE=3;

    Map<Integer,State> states=new HashMap<Integer,State>();

    public Car(){
        this.states.put(Car.ST_OFF, new Off());
        this.states.put(Car.ST_IDDLE, new Idle());
        this.states.put(Car.ST_DRIVE, new Drive());
        this.states.put(Car.ST_REVERSE, new Reverse()); 
        this.state=this.states.get(Car.ST_OFF);
    }

    private abstract class State{

        protected List<Integer> nextStates = new ArrayList<Integer>();

        public abstract void handle();
        public abstract void change();

        protected State promptForState(String prompt){
            State s = state;
            String word = JOptionPane.showInputDialog(prompt);
            int ch = -1;
            try {
                ch = Integer.parseInt(word);
            }catch (NumberFormatException e) {
            }   

            if (this.nextStates.contains(ch)){
                s=states.get(ch);
            } else {
                System.out.println("Invalid option");
            }
            return s;               
        }       

    }

    private class Off extends State{

        public Off(){ 
            super.nextStates.add(Car.ST_IDDLE);             
        }

        public void handle() { System.out.println("Stopped");}

        public void change() {
            state = this.promptForState("Stopped, iddle="+Car.ST_IDDLE+": ");
        }

    }

    private class Idle extends State{
        private List<Integer> nextStates = new ArrayList<Integer>();
        public Idle(){
            super.nextStates.add(Car.ST_DRIVE);
            super.nextStates.add(Car.ST_REVERSE);
            super.nextStates.add(Car.ST_OFF);       
        }

        public void handle() {  System.out.println("Idling");}

        public void change() { 
            state=this.promptForState("Idling, enter 0=off 2=drive 3=reverse: ");
        }

    }

    private class Drive extends State{

        private List<Integer> nextStates = new ArrayList<Integer>();
        public Drive(){
            super.nextStates.add(Car.ST_IDDLE);
        }       
        public void handle() {System.out.println("Driving");}

        public void change() {
            state=this.promptForState("Idling, enter 1=iddle: ");
        }       
    }

    private class Reverse extends State{
        private List<Integer> nextStates = new ArrayList<Integer>();
        public Reverse(){ 
            super.nextStates.add(Car.ST_IDDLE);
        }           
        public void handle() {System.out.println("Reversing");} 

        public void change() {
            state = this.promptForState("Reversing, enter 1=iddle: ");
        }       
    }

    public void request(){
        this.state.handle();
    }

    public void changeState(){
        this.state.change();
    }

    public static void main (String args[]){
        Car c = new Car();
        c.request(); //car is stopped
        c.changeState();
        c.request(); // car is iddling
        c.changeState(); // prompts for next state
        c.request(); 
        c.changeState();
        c.request();    
        c.changeState();
        c.request();        
    }

}
  • 1
    I really like this. While I appreciate the top answer and it's defence of switch statements (I'll forever remember that now), I really really like the idea of this pattern. Thank you – PythonNewb Sep 1 '16 at 12:04
  • @PythonNewb Did you run it? – Tulains Córdova Sep 1 '16 at 12:12
  • Yes, it works perfectly. The implementation will be slightly different for the code I have, but the general idea is great. I think I might consider moving the state classes out of the enclosing class though. – PythonNewb Sep 1 '16 at 12:25
  • 1
    @PythonNewb I changed the code to a shorter version reutilizing the change state/prompt for input logic using an abstract class instead of an interface. It's 20 lines shorter but I tested and works the same. You can always get the older, longer version looking at the edit history. – Tulains Córdova Sep 1 '16 at 13:44
  • 1
    @Caleth As a matter of fact I wrote it like this because I usually do that in real life, i.e., store interchangeable pieces in maps and getting them based of ID's loaded from a parameter file. Usually what I store in the maps are not the objects themselves but their creators if the objects are expensive or have a lot of non-static state. – Tulains Córdova Jun 7 '17 at 9:37
16

switch statements are bad

It's this kind of over simplification that gives object oriented programming a bad name. Using if is just as "bad" as using a switch statement. Either way you're not polymorphically dispatching.

If you must have a rule that fits in a sound bite try this one:

Switch statements become very bad the moment you have two copies of them.

A switch statement that is not duplicated anywhere else in the code base can sometimes manage to not be evil. If the cases are not public, but are encapsulated, it's really nobody's else's business. Especially if you know how and when to refactor it into classes. Just because you can doesn't mean you have to. It's because you can that it's less critical to do it now.

If you find yourself trying to shove more and more stuff into the switch statement, spreading knowledge of cases around, or wishing it wasn't so evil to just make a copy of it then it's time to refactor the cases into separate classes.

If you have time to read more than a few sound bites about refactoring switch statements c2 has a very well balanced page about the switch statement smell.

Even in OOP code, not every switch is bad. It's how you're using it, and why.

2

The car is a type of state machine. Switch statements are the simplest way to implement a state machine lacking super states and sub states.

2

Switch statements are not bad. Do not listen to people who say things like "switch statments are bad"! Some particular uses of switch statements are an antipattern, like using switch to emulate subclassing. (But you can also implement this antipattern with if's, so i guess if's are bad too!).

Your implementation sounds fine. You are correct is will get hard to maintain if you add many more states. But this is not just an issue of implementation - having an object with many states with different behavior is itself a problem. Imaging your car has 25 states were each exhibit different behavior and different rules for state transitions. Just to specify and document this behavior would be an enormous task. You will have thousands of state-transition rules! The size of the switch would just be a symptom of a larger problem. So if possible avoid going down this road.

A possible remedy is to break the state into independent substates. For example, is REVERSE really a distinct state from DRIVE? Perhaps the car states could be broken up in two: Engine state (OFF, IDLE, DRIVE) and direction (FORWARD,REVERSE). Engine state and direction will probably be mostly independent, so you reduce logic duplication and state transition rules. More objects with fewer states are a lot easier to manage than a single object with numerous states.

1

In your example, cars are simply state machines in the classic computer science sense. They have a small, well-defined set of states and some kind of state transition logic.

My first suggestion is to consider breaking out the transition logic into its own function (or class, if your language doesn't support first-class functions).

My second suggestion is to consider breaking out the transition logic into the state itself, which would have its own function (or class, if your language doesn't support first-class functions).

In either scheme, the process for transitioning state would look something like this:

mycar.transition()

or

mycar.state.transition()

The second could, of course, trivially be wrapped in the car class to look like the first.

In both scenarios, adding a new state (say, DRAFTING), would only involve adding a new type of state object and changing the objects that specifically switch to the new state.

0

It depends on how large the switch might be.

In your example, I think a switch is OK since there aren't really any other state I can think of that your Car could have, so it wouldn't get bigger over time.

If the only issue is having a large switch where each case has lots of instructions, then just make distinct private methods for each.

Sometimes people suggest the state design pattern, but it's more appropriate when you deal with complex logic, and states making different business decisions for many distinct operations. Otherwise, simple problems should have simple solutions.

In some scenarios, you could have methods that only perform tasks when the state is A or B, but not C or D, or have multiple methods with very simple operations that depend on the state. Then one or several switch statements would be better.

0

This sounds like an old-school state machine of the sort that was used before anyone did Object Oriented programming, let alone Design Patterns. It can be implemented in any language that has switch statements, such as C.

As others have said, there's nothing inherently wrong with switch statements. The alternatives are often more complicated and harder to understand.

Unless the number of switch cases becomes ridiculously large, the thing can remain quite manageable. The first step in keeping it readable is to replace the code in each case with a function call to implement the state's behaviour.

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.