views:

153

answers:

5

I have an application that has multiple states, with each state responding to input differently.

The initial implementation was done with a big switch statement, which I refactored using the state pattern (at least, I think it's the state pattern. I'm kind of new to using design patterns, so I tend to get them confused) -

class App {

  public:
    static App * getInstance();
    void addState(int state_id, AppState * state) { _states[state_id] = state; }
    void setCurrentState(int state_id) { _current_state = _states[state_id]; }

  private:
    App()
    ~App();
    std::map<int, AppState *> _states;
    AppState * _current_state;
    static App * _instance;
}

class AppState {

  public:
    virtual void handleInput() = 0;
    virtual ~AppState();

  protected:
    AppState();

}

Currently, each state is polling the OS for input, and acting accordingly. This means each concrete state has a huge switch statement with a case for each valid keypress. Some cases call functions, and other cases issue state changes by using App::setCurrentState(newstate). The catch is that a key that does something in one state may not do anything (or in rare circumstances, may do something different) in another state.

Okay, I think that's the pertinent background. Here's the actual question(s) -

First, what's the best way to eliminate the huge switch statements in the concrete states? This question suggests the command pattern, but I don't understand how I would use it here. Can someone help explain it, or suggest another solution?

As a side note, I've considered (and am not opposed to) the idea of letting the App class do the polling of the OS, and then pass inputs to _current_state->handleInput. In fact, something tells me that I'll want to do this as part of the refactoring. I just haven't done it yet.

Second, state changes are made by calling App::setCurrentState(newstate). I realize that this is akin to using globals, but I'm not sure of a better way to do it. My main goal is to be able to add states without modifying the App class. Suggestions would be welcome here as well.

A: 

I wrote a library where I refactored a lot of the state stuff. It's clean and quite OO, and doesn't really add a lot of overhead, but it's quite a different way to code a state machine.

It includes a couple tests, if nothing else they might give you some ideas.

You're welcome to look at it if you like: http://code.google.com/p/state-machine/

Bill K
I'll check it out. Thanks.
Bill B
A: 

If you are able to capture all OS input in a class, then you can have one object which listens for input, and use the chain of responsibility pattern to notify specific actions of the OS input.

eulerfx
Encapsulating the input to one class would be easy enough. Though, I don't understand how chain of responsibility would work, since only one state will be active at a time.
Bill B
A: 

You look at things as:

I have a static container for states (your App), and a lot of states (you AppState) which can contain data and only one handler.

Instead look at it as:

I have a one StateMachine class. (I may or may not have many instances of it.) This holds the data needed to interact with the outside world. Is also contains a static set of eventHandlers, one for each state. These eventHandler classes contain no data.

class StateMachine {
public:
    void handleInput() { //there is now only one dispatcher
        if( world.doingInput1() )
            _current_state->handleInput1( *this );

        else if( world.doingInput2() )
            _current_state->handleInput2( *this, world.get_Input2Argument() );

        //...
    }

    //the states, just a set of event handlers
    static const State& state1;
    static const State& state2;
    //...

    StateMachine( OutsideWorld& world )
       :world( world )  
    {
        setCurrentState( StateMachine::state1 );
    }

    void setCurrentState( const State& state ) { _current_state = &state; }

    OutsidWorld& world;
private:
    State* _current_state;
};

class State {
public:
    //virtual ~State(); //no resources so no cleanup
    virtual void handleInput1( StateMachine& sm ) const {};
    virtual void handleInput2( StateMachine& sm, int myParam ) const {};
    //...
};

class State1 {
public:
    //define the ones that actually do stuff
    virtual void handleInput1( StateMachine& sm ) const { 
       sm.world.DoSomething();
       sm.setCurrentState( StateMachine::state27 );
    }
    virtual void handleInput27( StateMachine& sm, int myParam ) const { 
       sm.world.DoSomethingElse( myParam );
    };
};
const State& StateMachine::state1 = *new State1();

//... more states
jyoung
Thanks for your input. Your exact approach won't work for me (for example, I can't define the states to be used at compile time), but it's validated for me that I'm moving in the right direction.However, if state-specific data shouldn't be stored in the state, where should it be stored?
Bill B
Wouldn't you want class State1 to be derived from class State?
Dan
+1  A: 

I've refactored things a bit -

I've eliminated the direct calls to App::setCurrentState by requiring that a pointer to the state machine (App) be passed to the AppState constructor. That way, all necessary calls can be made via that pointer.

I've added a parameter to handleInput, and made it so that App does the OS input polling and passes any input to the current state.

The new code looks like this -

class App {

  public:
    static App * getInstance();
    void addState(int state_id, AppState * state) { _states[state_id] = state; }
    void setCurrentState(int state_id) { _current_state = _states[state_id]; }

  private:
    App()
    ~App();
    std::map<int, AppState *> _states;
    AppState * _current_state;
    static App * _instance;
}

class AppState {

  public:
    virtual void handleInput(int keycode) = 0;
    virtual ~AppState();

  protected:
    AppState(App * app);
    AppState * _app;

}

So this still leaves in every state, a large switch statement that converts the keypresses to state-specific actions. I suppose I could replace the switch with a map of keys to actions, but I'm still wondering if there's a better way.

Bill B
+1  A: 

Given your refactoring, it looks like the question now is how to reduce the amount of keycode parsing code that will be duplicated across your various concrete AppState implementations. As you mention, this leads to multiple switch statements which select which code to call to handle the keystroke input.

Depending on how performance critical this code is, you can separate that keycode decoding logic into a processInput(int keycode) method in App (or as a concrete method in AppState) and create a set of handle*Pressed() functions in your AppState classes. Depending on how many types of keystrokes you are looking at handling, this might be reasonable, or it might lead to far too many methods to implement.

Suppressingfire
OK, so I realize I'm late in accepting this... I kind of forgot about it. At any rate, this is the approach I wound up taking. Thanks!
Bill B