search

yohang / Finite

A Simple PHP Finite State Machine
http://yohan.giarel.li/Finite
MIT License
1.31k stars 188 forks source link

Which direction was this library going? #95

Open RonRademaker opened 8 years ago

RonRademaker commented 8 years ago

I moved from the winzou/statemachine to this one because I wanted to add an interface to my stateful objects and have methods manipulate these. Now I've been looking at what has been committed into this library and I see those things have been changed in master. It looks like it was moving towards the winzou/statemachine approach.

I'd like to known the bigger picture about where this statemachine library is going. For now I'm planning to fork it at the last 1.0 release and apply my own patches from there.

Revisor commented 8 years ago

I copy your comment from #96

basically the StatefulInterface has been replaced by a StateAccessor that by default uses a public variable state. It's more like the winzou/statemachine I think. I started using and went back to 1.0 fast (it's a matter of taste of course).

As far as I understand it from the discussion at #39, the StateAccessor has been added so that one object can have multiple state graphs. The previous solution allows only for one state graph at a time.

The StatefulInterface stays for people who just want one state graph.

That sounds reasonable to me.

RonRademaker commented 8 years ago

I think it would have been better if an interface 'MultiGraphStatefuleInterface' was added that extends from StatefulInterface, this interface would require implementing something like setStateAccessor(StateAccessorInterface $stateAccessor, $graph). That way the support can be added BC and stateful objects are still easily recognizable in the application. The object would be responsible for returning it's correct state for a particular graph.

Revisor commented 8 years ago

I understand your points. Frankly it sounds like a good idea to unite multigraph and single graph states under the umbrella of one interface, and to have the interface mark stateful objects. We also wouldn't have to lose the little type hinting we can have in PHP (see #39).

After all, "one is just a special case of many".

Would you care to draw up a solution and offer it for code review in a pull request?

RonRademaker commented 8 years ago

Yes, I'll create a PR.

yohang commented 8 years ago

@RonRademaker I partially agree with you. When creating Finite, having a StatefulInterface was in the foundations of this library.

I think I'll work on an interface-based multi-graph solution for 2.0 (aka BC break allowed), but i'll keep the current solution for 1.1, which allow you to use the interface for single-graph.

RonRademaker commented 8 years ago

1.1 will break BC in it's current condition, for example because all the typehints from StateMachine are gone (so anyone who has extended the class will have a broken package after an upgrade).

yohang commented 8 years ago

@RonRademaker in fact, yes, you're right. But it seems "acceptable" to me as this is not a standard and documented thing.