Member function state machine

Question

For various reasons, I wrote this. For one, I don't like to wrap every single function in a separate class, which is too verbose.

It's a state machine based on GoTW 57 function pointer wrapping and CRTP to make things readable, at least in client code parts.

template <class Derived>
struct StateMachine {
    typedef Derived Self;
    typedef StateMachine<Derived> Base;

    //http://www.gotw.ca/gotw/057.htm
    struct Func;
    typedef Func (Derived::*UpdateFuncPtr)();
    struct Func {
        UpdateFuncPtr updateFunc;
        Func(UpdateFuncPtr ptr) : updateFunc(ptr) {}
    };

    //actual statemachine .. can be as complex as needed
    Func currentState;
    StateMachine( Func init) : currentState(init) {}
    void update() {
        UpdateFuncPtr p = currentState.updateFunc;
        Self * pThis = (Self *)this;
        Func newPtr = (pThis->*p)();
        if(newPtr.updateFunc)
            currentState = newPtr;
        }
};

The client looks like this:

struct DemoSM : StateMachine<DemoSM> {
    int check; //just checking that *this is sane
    DemoSM() : Base( &Self::update1 ) {
        check = 42;
    }
private:
    Func update1() {
        log("update1 called, check %d\n\r",check);
        return &Self::update2;
    }
    Func update2() {
        log("update called\n\r");
        return NULL;
    }
};

And just checking:

DemoSM machine;
machine.update();
machine.update();

Any critique or suggestions for improvements or gotchas welcome. The core principle is that the state machine functions themselves code, procedurally (not declaratively in a table), how the transitions get triggered - by returning the function that needs to be called next.

This can be made marginally more interesting with separate "update" and "state transition" functions maybe with different signatures.

I could not get GoTW suggested operator() to work for taking pointer to members. C++ is insistent on &Class::member; syntax there.

Answer 1

CRTP helpers

When using CRTP, it is common to write derived methods in the base class to avoid having to deal with the this pointer and to use reference semantics instead:

Derived& derived() {
    return static_cast<Derived&>(*this);
}

Derived const& derived() const {
    return static_cast<const Derived&>(*this);
}

Thanks to these functions, your update method becomes:

void update() {
    UpdateFuncPtr p = currentState.updateFunc;
    Func newPtr = (derived().*p)();
    if(newPtr.updateFunc)
        currentState = newPtr;
    }

And it is now easier to have the base class call methods from the derived type without having to deal with the this pointer.

Improving Func

If you want to impose more responsibility on Func and less on StateMachine itself, you can still add operator UpdateFuncPtr() const to Func to allow easy conversions to and from the class:

operator UpdateFuncPtr() const {
    return updateFunc;
}

That will allow you simply again the method update:

void update() {
    UpdateFuncPtr p = currentState;
    Func newPtr = (derived().*p)();
    if(newPtr)
        currentState = newPtr;
    }

The implicit conversion to `UpdateFuncPtr` is also used for the boolean conversion needed to check whether `newPtr` is `NULL` or not. Be careful though, implicit conversions are often the source of many hidden errors. Implementing `operator()` in `Func` would be quite difficult since it would mean that `Func` must know the instance of `StateMachine` on which you wish to call the methods. All in all, it is easier to keep the `this` pointer out of `Func`.

Access modifiers

There are several parts of the base class that you do not need in the derived class. You could make them private:

• UpdateFuncPtr
• currentState
• The derived methods if you choose to use them

Moreover, there are even more things that the final users of the state machine don't need to know. Actually, the only need to know about update, so almost everything else could be made protected.

Stylistic tidbits

There are a few more details that can be improved with your code:

• Try to always use constructor initialization lists when you can (you may have forgotten it in this particular case because it's only an example):

  DemoSM() :
      Base( &Self::update1 ),
      check( 42 )
  { }
  

• Your brace indentation in update is really misleading. When modifying the code, I found it hard to read since the last brace seems to match the if while it actually closes the function block. To reflect that, this last brace should be aligned with the function signature, like every other function in your code.

• If possible, try to avoid if blocks without braces. While it may be easier to write when there is only one line, your code proves that it can hide subtle bugs: at first, I thought that the last closing brace closed the if and I tried to add lines to it (to perform some checks) while I wasn't actually adding to the if. Had you used braces for the if, I wouldn't have had this problem.

• You don't seem to be very consistent with regard to parenthesis and spaces. You might want to choose some guideline and apply it everywhere.