Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have designed a simple state machine for the following diagram:

enter image description here

I am seeking improvements from the community. I especially don't like two things:

  1. When calling performAction I wan't to send only the action without the callback.
  2. And maybe remove the Action enum from the Engine class.

For simplicity everything is added in one .cpp file:

#include <iostream>
#include <utility>
#include <map>

typedef enum {
    Idle,
    Start,
    Stop,
    ChangeSpeed
}State;


class Engine
{
public:
    typedef enum {
        SetSpeed,
        Halt
    }Action;

    typedef void (Engine::*DoAction)(void);

    Engine() {
        std::cout << "Engine()" << std::endl;
    }
    void setSpeed() {
        std::cout << "Engine::setSpeed()" << std::endl;
    }
    void halt() {
        std::cout << "Engine::halt()" << std::endl;
    }
};

struct ActionCallback
{
    Engine::Action         action;
    Engine::DoAction       callback;
    ActionCallback(Engine::Action a, Engine::DoAction c):action(a), callback(c){}
};

struct StateAction {
    State state_;
    Engine::Action action_;
    StateAction(State state, Engine::Action action):state_(state), action_(action){}
    friend bool operator<(const StateAction& sta1, const StateAction& sta2) {
        return (sta1.state_ < sta2.state_) ||
        (sta1.state_ == sta2.state_ &&
         sta1.action_ < sta2.action_);
    }
};

class StateMachine {
public:
    typedef std::map<StateAction, State> StateDiagram;


    StateMachine(Engine &engine):engine_(engine) {
        stateDiag_[StateAction(Idle, Engine::SetSpeed)] = Start;
        stateDiag_[StateAction(Start, Engine::SetSpeed)] = ChangeSpeed;
        stateDiag_[StateAction(ChangeSpeed, Engine::SetSpeed)] = ChangeSpeed;
        stateDiag_[StateAction(ChangeSpeed, Engine::Halt)] = Stop;
        stateDiag_[StateAction(Start, Engine::Halt)] = Stop;
        //
        //  current state is initialized to Idle
        //
        currentState_ = Idle;
        std::cout << "State machine initial state: " << currentState_ << std::endl;
    }
    void performAction(ActionCallback actionCallback) {
        //
        //  The state machine "state" is: currentState
        //
        StateAction stateAction(currentState_, actionCallback.action);

        if (stateDiag_.find(stateAction) == stateDiag_.end()) {
            //
            //  Invalid state transition
            //
            std::cout << "You cannot perform action: "
            << actionCallback.action
            << " from state: "
            << currentState_
            << ", invalid transition!!!"
            << std::endl;
            return;
        }
        State oldState = currentState_;
        //
        //  Execute the action and update the state machine
        //
        (engine_.*actionCallback.callback)();
        currentState_ = stateDiag_[stateAction];

        std::cout << "State changed:\n\tOld state: " << oldState << "\n\tNew state: " << currentState_ << std::endl;
        //
        //  If current state is: Stop change state to: Idle
        //
        if (Stop == currentState_) {
            oldState = currentState_;
            currentState_ = Idle;
            std::cout << "State changed:\n\tOld state: " << oldState << "\n\tNew state: " << currentState_ << std::endl;
        }


    }
    State getInitialState() {
        return Idle;
    }

private:
    StateDiagram stateDiag_;
    State currentState_;
    Engine engine_;
};
int main()
{
    Engine engine;
    StateMachine stateMachine(engine);
    stateMachine.performAction(ActionCallback(Engine::SetSpeed, &Engine::setSpeed));
    stateMachine.performAction(ActionCallback(Engine::Halt, &Engine::halt));
    stateMachine.performAction(ActionCallback(Engine::Halt, &Engine::halt));
    return 0;
}
share|improve this question
    
Note that there are entire libraries dedicated to generate state machines, e.g. boost's MSM –  dyp May 18 at 22:20

2 Answers 2

up vote 3 down vote accepted

Express the state diagram as

std::map<std::pair<State, State>, ActionCallback>

Then performAction would require only one argument, along the lines of

void performAction(State nextState) {
    ActionCallback callback = stateDiagram[std::pair(currentState, nextState)];
    ...
share|improve this answer

That's typically how C enums are written, but it's done differently in C++. The typedef keyword is no longer needed, and the name is given after the enum keyword.

enum State {
    Idle,
    Start,
    Stop,
    ChangeSpeed
};
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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