alexpeachey / end_state

A State Machine implementation
MIT License
9 stars 5 forks source link

Bang Methods #13

Closed charlierudolph closed 10 years ago

charlierudolph commented 10 years ago

Not a big fan of the bang methods.

You offer both transition and transition! so the fact that triggering events or attempts to transition to states requires a bang at the end seems very odd to me.

I would think events would have both versions (with and without bang).

I think the interface to transition to a state directly by its name is a bad interface. If the example were more real world I think the weirdness would be more apparent. Take for example state_machine's example of a car. There would be a method parked! that transitions you to the parked state.

alexpeachey commented 10 years ago

So I like your points. I believe the thinking was that while we are creating a different architecture here we wanted to keep the interface similar to the other state machine gems out there.

You have a good point though and it probably doesn't make sense to transition with just the name of the desired state. Considering we do offer the event aliases for transitions there is no good reason to have this.

I also like the the idea of conforming the event aliases to match transition and transition!. Currently in practice the two known projects almost exclusively use the event and not the state to transition.

I'd like to get @brianvh feedback but I can see this making a lot of sense to go this route.

Since we haven't yet locked down to a 1.0, I think we can make this breaking change on the next point release. Unfortunately there is no graceful way to change the signature of the event aliases. All event aliases currently in use are the bang style so making this change means they all need to get changed when upgrading.

I believe the safest path would be to do the following:

I'd like to get this locked down to a version 1.0.0 here soon so perhaps this set of changes can be considered to be the last pre-1.0 changes. Then the 1.0.0 version can officially remove support for the transition name bang methods and remove the temporary macro and all deprecation warnings.

alexpeachey commented 10 years ago

While I think my plan is a good one and if this change was happening post-1.0 we would need to, I feel like with the current size of the user base and the added complexity this adds, it's better to just make the large breaking change.

brianvh commented 10 years ago

I'm on board with that (I'm also willing to help), even though it will result in a ton of changes to bring Versus back into line. We're pretty heavy users of end_state, with 2 very large StateMachine classes, 1 smaller one and at least 2 more that we have, in the works.

Fortunately, I can hold off on the additions, knowing that things will be in flux for a bit. :wink: