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

Which direction was this library going? #95

Closed
RonRademaker opened this issue Nov 20, 2015 · 7 comments
Closed

Which direction was this library going? #95

RonRademaker opened this issue Nov 20, 2015 · 7 comments
Labels
Milestone

Comments

@RonRademaker
Copy link
Contributor

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
Copy link

Revisor commented Nov 27, 2015

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
Copy link
Contributor Author

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
Copy link

Revisor commented Nov 27, 2015

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
Copy link
Contributor Author

Yes, I'll create a PR.

@yohang
Copy link
Owner

yohang commented Nov 30, 2015

@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.

@yohang yohang added this to the 2.0 milestone Nov 30, 2015
@RonRademaker
Copy link
Contributor Author

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
Copy link
Owner

yohang commented Nov 30, 2015

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

@yohang yohang added the future label Nov 30, 2015
@yohang yohang closed this as completed Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants