Adding basic guard implementation into transitiona and state check

[defrag]

@cordoval done

[yohang]

Hi,

Thanks for your contribution !

I'm ok for this feature, but there is some implementation details that i don't like.

• I think the guard shouldn't be typehinted as \Closure. Why can't it be any callable ? (Even if it should stay 5.3 compatible, we can use an is_callable test instead of typehint).
• As the state didn't know the transition instance, the guard implementation should live in the StateMachine. Your current implementation can result in weird behaviors, like $state->can('foobar') => true and $state->can($t/*transition instance representing the same transition*/) => false

[defrag]

About first point, yeah I guess this is fine, i have just ported this from my own project when i was using closures.

About second point: hmm, this is tricky since StateMachine have public assess to current state also. So $stateMachine->can('foobar') => false in new implementation, $stateMachine->getCurrentState()->can('foobar') => true. Current implementaion doesnt provide good solution I think. I thnk the solution would be to make State hold the references to objects, rather than transiton names.

What is your opinion on best way to handle this?

[yohang]

You're right, but sadly, for convenience reasons, the state can't hold references to transitions, this will create an dependency to the state machine inside the state, and so, cyclic dependencies.

As there is already an event-based guard in the ListenableStateMachine, that suffers of this side effect, maybe we should remove the can() method on the state, and keep this responsibility inside the StateMachine.

Or, maybe, this is the time to introduce a kind of Voter class that takes these decisions instead of the State machine and the state instance.

This will introduce a small BC-break, but as i'll tag the stable version soon, this is the time to do this.

[defrag]

Hmm yeah, removing can() from state would solve this issue, maybe thats the way to go. In similar ruby implementaions state doesnt check if the transition is correct, all the check methods were rather delegated to the state machine itself. I would go for that for simplicity.

[yohang]

So :+1: for me too.

I'll probably introduce a voter class in the future, but for the 1.0 release, let's focus on features :)

[defrag]

Okay, so i moved the check into state machine and squashed the commits, you can use this if You like :)

[yohang]

If you can fix the small 3 Typo/CS, I'll merge now

[defrag]

@yohang ive fixed the docs and applied cs fixer changes.

[yohang]

Merged.