Skip to content

add test for events-as-object #16

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.

Sign up for GitHub

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

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

add test for events-as-object #16

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions test/specs/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@ module.exports = function (promise) {
expect(fsm.current).to.be.equal('green');
});

it('should accept an Object for events property', function () {
StateMachine.Promise = promise;

var fsm = StateMachine({
events: {
Copy link
Owner

Choose a reason for hiding this comment

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

The events property of the configuration object is array in the other examples. Do you really need events as an object?

I have some state machines where this approach will not work as they have multiple events having the same name but starting from different states.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't need it this way, but it's less verbose if you don't have multiple events with the same name. I didn't even realize that was supported. So your call whether or not to support it--since it's trivial, why not?

Copy link
Owner

Choose a reason for hiding this comment

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

Although it is more readable to have the events as an object is not on par feature wise with having it as array (e.g. not being able to have multiple events with same name). For this reason we cannot say that these formats are interchangeable.

However, this is a library and we should maintain a level of flexibility. What about having an eventsLite configuration property that is merged into the events array? This way the users have a clear idea about what are the limitations of this approach.

Although I like the object notation, my main concern here is how to avoid confusion between the two. Do yu have a better way of addressing this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have some state machines where this approach will not work as they have multiple events having the same name but starting from different states.

I'd like to understand your use case better. Why do they have the same name? If event foo can be called in states a or b, does this event cause transition to state c in both cases? If so, you could specify multiple "from" states in the event definition, using the object notation:

{
  foo: {
    from: ['a', 'b'],
    to: 'c'
  }
}

But if it doesn't always transition to c (or have a conditional), I'm not sure why it has to be the same name?

Copy link
Owner

Choose a reason for hiding this comment

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

I have state machines that model the scoring systems in different sports. In this context the events and the state names are taken from the nomenclature of the respective sport. This is a simplified example:

[
 {name: 'end', from: 'Round1', to: ['Break', 'Final'], condition: ... },
 {name: 'end', from: 'Break', to: 'Round2' },
]

Using the array format I have the flexibility to do whatever is needed to implement the rules but also present them in a familiar form to the users.

start: {
from: 'one',
to: 'another'
}
}
});

expect(fsm.start).to.be.a('function');
});

it('should throw error on transition with array value for \'to\'', function (done) {
StateMachine.Promise = promise;

Expand Down