patternjavascriptMinor
Simple Javascript state management class
Viewed 0 times
simplejavascriptstatemanagementclass
Problem
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:
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]
//
- 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
selectedmight have parametersselectedPortandselectionContext
- 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]
//
Solution
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
Functionality-wise, this comment worries me a bit:
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:
But continuing with your current code, you've also got this line:
However, you don't need the name to check. Your
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.
For here, the caller can read/write whatever they want in that object, with no need for getters and setters. The
For handling transition callbacks, I'd storing them like so:
Which would make it easier to call them, when needed
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
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 t
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 isThis 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 stateHowever, you don't need the name to check. Your
inState function could simply be written asFSM.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 propertiesFor 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 t
Code Snippets
// Initialize to an arbitrary state, client can transition to
// whatever their desired initial state isfunction 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");// 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 stateFSM.prototype.inState = function(name) {
return this.state === this.states[name];
}stateMachine.properties() // => an object containing the current state's propertiesContext
StackExchange Code Review Q#58943, answer score: 4
Revisions (0)
No revisions yet.