next_state is set to early in process()

[GDmac]

Isn't the new current_state, inside the process() method, set to early? It is set before processing any callbacks that might change the state. This makes it hard inside an error callback (or any callback) to see the current state that triggered the callback, or even the one that triggered the error.

addTrans(FIRST,   START -> MIDDLE, CALL_1) 
addTrans(SECOND,  MIDDLE -> END,   CALL_2) 
default(ERROR,ErrorCallback)
AddAny(HALTED, HALTED, HaltedCallback)

function ErrorCallback($s,$p){
  // process error
  return 'halted';
}

e.g. if FIRST returns any 'state' other than one defined in a transition, then the ERROR callback is called, but the _current_state has already been changed in the process() method to transition(0) (MIDDLE). This makes it impossible to catch any unknown states inside a error callback.

Maybe i'm missing the point of Finite State Machines al together, but a solution (or call it a practical adaptation) is to change _current_state late in process(), only after any callback that might want to change current_state.

    function process($symbol)
    {
        $transition = $this->getTransition($symbol);

        /* If a valid array wasn't returned, return immediately. */
        if (!is_array($transition) || (count($transition) != 2)) {
            return; 
        }

        /* Update the current state to this transition's exit state. */
        // TO EARLY: $this->_currentState = $transition[0];
        $next_state = $transition[0];

        /* If an action for this transition has been specified, execute it. */
        if (!empty($transition[1])) {
            $state = call_user_func_array($transition[1],
                                          array($symbol, &$this->_payload));

            /* If a new state was returned, update the current state. */
            if (!empty($state) && is_string($state)) {
                //$this->_currentState = $state;
                $next_state = $state;
            }
        }

        // set the new _current_state here, after any callback that might change it
        $this->_currentState = $next_state;

    }

[GDmac]

Also, when callbacks call methods inside other classes / objects, its hard to find the callee FSM-class to call getCurrentState(). Together with the above changes to setting the state late, why not also pass the states to the callback? Make those callbacks a bit more aware of the state the machine is in?

        $state = call_user_func_array($transition[1],
             array($symbol, &$this->_payload, $this->_currentState, $next_state));

[thankgodness]

I quite agree with you

[jparise]

Thanks for suggesting this improvement @GDMac!

I'm afraid I can't pass more information to the callback functions without breaking the current API, so I'm not going to act on your second suggestion at this time.

[GDmac]

Yes, you totally can @jparise, the extra parameters are at the end, so applications that don't look at third and forth parameter just ignore it, and "newer" implementations can see if the arguments are present.

However, this suggestion is almost 3 years old (2012) :-) currently (2015) i'ld probably wouldn't want to clutter the parameter set and instead go with a single third argument, something like array $parameterbag which can transport data and maybe even $this around.

[jparise]

You're right, @GDmac. I had forgotten that PHP handles function arguments that way.

I extended the callbacks to accept the full set of arguments (instead of moving to a single array payload) to maintain compatibility.

And yes, I was unfortunately preoccupied enough over the last three years such that I never sat down to work this out properly. :)