3
\$\begingroup\$

So I needed very basic state management and notification for a small game-like thing I'm building. I decided to implement something like a finite state machine (but not quite, it doesn't transition upon events, but instead is told to transition). This is mostly used for changing visual behavior and animation or whatnot, so the most important ability of a client is to simply check which state it is in. I had a few objectives:

  • There needs to be a finite list of states configured at creation, and I as the user need to be warned if I try to enter a state that doesn't exist
  • States need to have parameters that hold auxiliary information. For example, a state called selected might have parameters selectedPort and selectionContext
  • I need basic event handling, so that other parts of the application can be notified upon entering or leaving a state

As far as the implementation, the class holds an objects of states, who themselves are objects that contain their own parameters

Here's the class I threw together:

function FSM(states) {
    this.states = states

    // Set the _fsmName of each state to its name in the states object
    // This makes it much easier to check if we're in a particular state
    for (var prop in this.states) {
        this.states[prop]._fsmName = prop
    }

    // Initialize to an arbitrary state, client can transition to 
    // whatever their desired initial state is
    this.state = this.states[Object.keys(this.states)[0]]

    this.cbReg = {}
}

FSM.prototype.transition = function(next) {

    for (var prop in this.states) {
        if (prop == next) {

            // Handle onLeave subscribers
            if (this.state._fsmName in this.cbReg) {
                this.cbReg[this.state._fsmName].forEach(function(val, ind, arr) {
                    if (val.when == "onLeave") {
                        val.callback()
                    }
                })
            }

            this.state = this.states[prop]

            // Handle onEnter subscribers
            if (this.state._fsmName in this.cbReg) {
                this.cbReg[this.state._fsmName].forEach(function(val, ind, arr) {
                    if (val.when == "onEnter") {
                        val.callback()
                    }
                })
            }

            return
        }
    }
    throw "FSM State doesn't exist"
}

FSM.prototype.inState = function(st) {
    return (this.state._fsmName == st)
}

FSM.prototype.get = function(p) {

    // Try to get a state parameter, but throw if the
    // parameter doesn't exist
    if (p in this.state) {
        return this.state[p]
    } else {
        throw "FSM Invalid Parameter"
    }
}

FSM.prototype.set = function(p, v) {

    // Try to set a state parameter, but throw if the
    // parameter doesn't exist, instead of silently adding it
    if (p in this.state) {
        this.state[p] = v
    } else {
        throw "FSM Invalid Parameter"
    }
}

FSM.prototype.register = function(stateName, when, cb) {

    // Register a callback upon entering or leaving a state

    if (!(stateName in this.states)) {
        throw "FSM State doesn't exist"
    }
    if ((when != "onEnter") && (when != "onLeave")) {
        throw "FSM Invalid callback time specifier"
    }

    if (!(stateName in this.cbReg)) {
        this.cbReg[stateName] = []
    }

    this.cbReg[stateName].push({
        when: when,
        callback: cb
    })
}

A client might use it like:

this.SM = new FSM({
    normal: {},
    selected: {
        selectedPort: null
    }
})

this.SM.transition("normal")

this.SM.register("selected", "onEnter", function() { ... })

...

if (this.SM.inState("normal"))

...

if (this.SM.inState("selected")) {
    var p = this.SM.get("selectedPort")
    ...
}

A few things:

  • Existing FSM libraries seemed much more focuses on managing state in async situations, like callbacks from web requests and so forth. Are there any libraries that implement a simpler model, and particularly with the existence of auxiliary state information?
  • Any obvious problems with functionality or style in this code?
  • Are there other design patterns that might accomplish my intent here?
\$\endgroup\$
4
\$\begingroup\$

I can't find a semicolon anywhere in your code. Sure, it'll work without them, but it's still a bad habit. Sooner or later, you'll write some lines of code that do require semicolons to parse correctly, or you'll feed the script to a minifier that just removes whitespace, and all of sudden things are broken. Basically, while JS does do automatic semicolon insertion, relying on it is a little iffy in my opinion. The language has semicolons for a reason.

Otherwise, the code looks well-formatted. Some names could be more descriptive, though. For instance prop in your for...in loops might more descriptively be named stateName or simply state. "Prop" isn't terribly descriptive (later you have single-letter variables which are even more opaque). You've also got some variables you're not using at all like ind and arr in your forEach callbacks. If you're not using them, leave them out (and if you need them, ind should just be i or index as those are the most conventional names for an "index" variable.)

Functionality-wise, this comment worries me a bit:

// Initialize to an arbitrary state, client can transition to 
// whatever their desired initial state is

This basically means that users must make two calls to fully set up the state machine. If they don't, the initial state will be arbitrary. And if they do, but they've already registered transition callbacks, those callbacks will fire even though you're just trying to set up the state machine). So you may get some unexpected, spurious state-changes, simply as a result of setting up the state machine.

Of course, as long as you use an object to hold the states, there's no guaranteed ordering, so there's no "first" state. I'd simply recommend not using an object, for this very reason.

Instead, you could do something like:

function FSM(stateNames /*, ... */) {
  this.states = Array.prototype.slice.call(arguments);
  if(!this.states.length) {
    throw new Error("No states given");
  }
  this.state = this.states[0]; // default to the first named state
  // ...
}

// ...

var stateMachine = FSM("idle", "working", "completed");

But continuing with your current code, you've also got this line:

// Set the _fsmName of each state to its name in the states object
// This makes it much easier to check if we're in a particular state

However, you don't need the name to check. Your inState function could simply be written as

FSM.prototype.inState = function(name) {
    return this.state === this.states[name];
}

and there'd be no need to change the object you've been passed. The caller might hold their own reference to that object, and won't expect it to change.

Now, the following is a bunch of suggestions for different approaches; sometimes quite different. I'm not saying this is the "one true way" to build things, it's just how I'd probably do it:

For handling associated properties, I'd propose something different: Simply return an object, and let the caller sort it out, instead of having getters and setters.

stateMachine.properties() // => an object containing the current state's properties

For here, the caller can read/write whatever they want in that object, with no need for getters and setters. The properties function could accept an optional argument in case you want properties for something other than the current state.

For handling transition callbacks, I'd storing them like so:

this.callbacks = {
  stateName: {
    enter: [callback, callback, ...],
    exit: [callback, callback, ...]
  }
};

Which would make it easier to call them, when needed

FSM.prototype.transition = function (targetState) {
  if(this.states.indexOf(targetState) === -1) {
    throw new Error("State '" + targetState + "' does not exist");
  }

  this.callbacks[this.state].exit.forEach(function (callback) { callback(); });
  this.state = targetState;
  this.callbacks[targetState].enter.forEach(function (callback) { callback(); });
}

Speaking of, it might be nice to pass the current and upcoming state names to the callbacks.

Lastly, it could be a good idea to skip the prototype functions in favor of dynamically adding functions in the constructor, or returning an object with the functions you want. For prototype functions to work, things like this.state have to be accessible, which means they're also accessible to the outside world. So I could just call stateMachine.state = 'foobar' and sidestep the transition function (and its checks and callbacks) entirely.

Below is an alternative implementation (again, this is just how I might do it - it's not to say that this is the only way):

function FSM(stateNames) {
  // keep variables local
  var states = {},
      currentState;

  stateNames = Array.prototype.slice.call(arguments);

  // check arguments
  if(!stateNames.length) {
    throw new Error("No states given");
  }

  // set initial state
  currentState = stateNames[0];

  // create objects to hold state properties and callbacks
  stateNames.forEach(function (name) {
    states[name] = {
      properties: {},
      callbacks: {
        enter: [],
        exit: []
      }
    };
  });

  // return an object with the public API. This lets the functions
  // access the "private" variables above with having to expose them
  // to the outside world, and lets us call either "new FSM(...)" or
  // just "FSM(...)" and get something useful

  return {
    // get the current state name
    state: function () {
      return currentState;
    },

    // return (a copy of) the list of state names
    states: function () {
      return stateNames.slice(0);
    },

    // transition to another state
    transition: function (targetState) {
      if(!states[targetState]) {
        throw new Error("State '" + targetState + "' does not exist");
      }

      states[currentState].callbacks.exit.forEach(function (callback) {
        callback(currentState, targetState);
      });

      currentState = targetState;

      states[currentState].callbacks.exit.forEach(function (callback) {
        callback(currentState, targetState);
      });
    },

    // get state properties (optional stateName argument)
    properties: function (stateName) {
      stateName = stateName || currentState;
      return states[stateName].properties;
    },

    // add a transition listener
    on: function (stateName, event, callback) {
      // this'll automatically throw an exception if the state or event doesn't exist
      states[stateName][event].push(callback);
    },

    // remove a transition listener
    off: function (stateName, event, callback) {
      var callbacks = states[stateName][event];

      for (var i = callbacks.length - 1 ; i >= 0 ; i++) {
        if(callbacks[i] === callback) {
          callbacks.splice(i, 1);
        }
      }
    }
  };
}
\$\endgroup\$
  • \$\begingroup\$ Excellent, +1, especially it might be nice to pass the current and upcoming state names to the callbacks. \$\endgroup\$ – konijn Aug 4 '14 at 13:04

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.