Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow "instance"-style usage #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boneskull
Copy link
Contributor

This needs tests, obviously, but it brings fsm-as-promised more inline with how these composable factories are intended to be used. PR #15 was simply a refactor; almost no actual API changes were involved.

Whenever you call a "stamp" (a composable factory function) created by Stampit, the first parameter is always going to be an object. The object returned is, by default, not the object you pass as the parameter. However, the object you pass (henceforth known as an "instance") is mixed in (via Object.assign(), _.assign(), etc) to the resulting object, e.g.:

const MyStamp = stampit({
  methods: {
    foo () {
      return this.bar;
    }
  }
});

const myObj = MyStamp({bar: 'baz'});
myObj.foo() // baz

fsm-as-promised's API allows mixing in of an instance via the second parameter; the first, of course, being simply the configuration.

With these changes, we can now compose freely with fsm-as-promised. Before this change, the following would not work as expected:

const myObj = StateMachine({
  foo: 'bar',
  events: {}, 
  callbacks: {}, 
  initial: 'start'});

myObj.foo // undefined

Above, the first parameter is used for configuration, then thrown away. To get this behavior, you'd need to do:

const myObj = StateMachine({
  events: {}, 
  callbacks: {}, 
  initial: 'start'}, 
  {
    foo: 'bar'
  });

myObj.foo // 'bar'

Now, the above is still supported, but the problem with this is that if you ever wanted to compose from StateMachine, you're in for pain, because the first parameter is thrown away.

const MyStamp = stampit({
  methods: {
    foo () {
      return this.bar;
    }
  }
}).compose(StateMachine);

const myObj = MyStamp({
  events: {}, 
  callbacks: {}, 
  initial: 'start'
});

myObj.foo() // TypeError: myObj.foo is not a function

Oops. Anyway, the old behavior is still supported with some trickery--and it will now "mix in" an EventEmitter into the second parameter (target) if it has no emit function--but now this works as expected:

const myObj = StateMachine({
  foo: 'bar',
  events: {}, 
  callbacks: {}, 
  initial: 'start'});

myObj.foo // 'bar'

This is accomplished by providing a second factory function (Target) which creates an instance of StateMachine internally, which sets everything up. StateMachine then exposes a mixin object for the Target to mix into itself via _.assign().

Of note, when you write:

var StateMachine = require('fsm-as-promised');

you are now no longer using the StateMachine factory; you're actually getting Target. For hacking's sake, to get the actual StateMachine factory, you'd do this:

var StateMachine = require('fsm-as-promised').StateMachine;

This change should be transparent to the user, unless they are vehemently opposed to having an EventEmitter mixed in to their target object (second parameter, as per the old API).

EventEmittable is a composable factory function (a "stamp") wrapping EventEmitter. To use it, simply call .compose(EventEmittable) upon any stamp.

init: function init (context) {
var target = _.first(context.args) || this;

_.defaults(target, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this whole thing will prioritize the configuration in the first parameter, but if it's not passed, then it'll try to take it from the second parameter, and if it's not there either, just use the defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the defaults are defined in props, but if target is not this, then they aren't going to be used, which is why we have to do it twice. Unless there's a really good reason to support the second parameter, maybe we should drop that functionality, because it's a bit cumbersome. In other words--we can now mix any object into a state machine in addition to mixing a state machine into any object, but do we need both?

@boneskull
Copy link
Contributor Author

Once you've considered the ideas herein I'll add proper tests, if you're to go that way.

@vstirbu
Copy link
Owner

vstirbu commented Mar 20, 2016

In my view the configuration object (or the first parameter) is the specification or blueprint of the state machine. Among the properties of the configuration, the most important is the events property as this one describes the behaviour, regardless of the implementation. init informs the initial state and it might be dynamic in case we add a feature to suspend/resume the state machine. final is just sugar in case an observer would monitor when things are done. callbacks are just implementation.

These are the basic features of the configuration, so I don't quite understand why there are more properties in the configuration if they are not strictly related to the behaviour of the state machine (e.g. foo)?

If there is some other data that used by the state machine instance it can be provided in a closure or as the second parameter.

Stampit is new to me so I might have missed some of the point you made...

On the other hand, this PR is quite large and it might be split into smaller pieces, or just itemise the features that it brings in, such as mixing event emitter into the instance if that does not have it, configure the promise lib via usePromise...

@boneskull
Copy link
Contributor Author

I'll try to explain Stampit as best as I can below.

The most basic Stamp (a composable factory function), is the following:

// Ex.1
const MyStamp = stampit();
const myObj = MyStamp(); // alternatively, MyStamp({})
myObj; // {}

Example 1 above is functionally equivalent to:

// Ex.2
const myObj = Object.create({});
myObj; // {}

Both examples 1 and 2 are trivial and useless. If we want to make a factory function ourselves which actually does something, we could write:

// Ex.3
const prototype = {
  foo() {
    return this.value;
  }
};
function factory (instance) {
  return Object.assign(Object.create(prototype), instance);
}
const myObj = factory({value: 'bar'});
myObj.foo(); // bar

Stampit provides the ability to write example 3 as:

// Ex.4
const MyStamp = stampit({
  methods: {
    foo() {
      return this.value;
    }
  }
});
const myObj = MyStamp({value: 'bar'});
myObj.foo(); // bar

In a nutshell, Stampit provides more bells and whistles around example 3. What is common between these two implementations is that the first parameter to the factory function is "mixed-in" to the resulting object--the resulting object is never strictly equal to the first param passed into each function.

So then, your comment above:

In my view the configuration object (or the first parameter) is the specification or blueprint of the state machine.

...poses a problem, because it is directly at odds with Stampit's API, if I take it to mean what I think you mean. I have some questions, then

What is a "state machine"? Assuming the object returned is by the function a "state machine" (even though it may really be just a proxy):

  • Is it an instance of a class? (No)
  • Is it an object which is returned by function? (Yes)
  • Is it an object which is created by function? (Sometimes)
  • If the function created the returned object, is that object a "state machine"? (Yes)
  • If the function did not create the returned object, then is the returned object a "state machine"? (Sure..?)
  • If the function did not create the returned object, then are any other properties within the returned object part of the "state machine"?
  • Furthermore, if someone were to extend the function in some way, would the resulting object be a "state machine?"

I didn't answer the last two, because I don't know. I do know that providing a function which may or may not have side-effects is not the most straightforward API.

If I am going to make an argument, it's that:

  1. The function exported by fsm-as-promised should only create new objects.
  2. Since you can only create new objects, then, the new objects should be allowed to have arbitrary properties.

To your argument, I see where you're coming from--the properties passed as the first parameter should "configure" the resulting object, but not be properties of it. To make this work with the Stampit API, the most straightforward solution would be to pass the config as the second parameter, leaving the first for any properties which are to be mixed-in to the returned object (you see the first parameter as the "configuration", whereas I'm coming from a place where that first parameter contains object properties).

OK, so that's a solution, but it's not very elegant, nor is it user-friendly if you don't have any extra parameters!

An API such as this may be easier to stomach, however (and this is trivially implemented, mind you--it mimics my own wrapper of fsm-as-promised):

// Ex.5
const TrafficLight = StateMachine
  .methods({
    slamBrakes() {
      // stuff, returns Promise
    },
    speedUp() {
      // other stuff, returns Promise
    }
  })
  .event({
    name: 'go',
    to: 'green'
  })
  .event({
    name: 'slowDown',
    to: 'yellow',
    from: 'green'
  })
  .event({
    name: 'stop',
    to: 'red',
    from: 'yellow'
  })
  .initial('go')
  .final('stop')
  .callback('enterYellow', function () {
    return this.floorIt();
  })
  .callback('leaveYellow', function () {
    return this.slamBrakes();
  });

const lightA = TrafficLight({name: 'A'});
const lightB = TrafficLight({name: 'B'});
const lightNoName = TrafficLight();

Above, StateMachine() creates an object that supports the current behavior event, callback, and transition behavior. We extend StateMachine(), through its static methods, to custom-tailor the resulting factory function to our use case.

Using an API like this--even supporting the config-as-second-parameter idea--we can avoid exposing the state machine's guts to the user. Either way, the API must change if you are to fully leverage Stampit's power and flexibility; otherwise if I want to write a plugin or fiddle with something, I'm back to monkeypatching.

Please let me know what you think before I modify the PR further.

@vstirbu
Copy link
Owner

vstirbu commented Mar 21, 2016

If I am going to make an argument, it's that:

  • The function exported by fsm-as-promised should only create new objects.
  • Since you can only create new objects, then, the new objects should be allowed to have arbitrary properties.

Yes, I agree with both statements.

The API in example 5 is a good compromise that addresses my concerns and also plays nice with Stampit. I like a lot the ability to create the instance independent state machine factory, which can be used to create the actual instances that can be configured via the provided object. I have use cases in which I provide only the configuration so it would be a bit odd to have an API in which the first parameter is null or undefined.

I would assume that the state machine factory function can be created using the chaining as in ex 5 but also by providing the "classic" configuration object from the current API.

I assume that the .event function can handle conditional events too :).

@boneskull
Copy link
Contributor Author

Sounds great. I'll modify this accordingly with the proposed API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants