Add ability to specify from/to arrays within transitions #28
Conversation
RobvH
force-pushed
the
State-plus-transition-keys-destination
branch
from
93d072b
to
f4d7719
Compare
Previously the state machine machine was able to express multiple source states for a transition, but was unable to define the complete source + transition = destination. Consider processing + begin_checkpoint = checkpointing. However, for the begin_checkpoint transition a different destination state must be defined for another source state. E.g. shutting_down + begin_checkpoint = final_checkpointing, etc. No backwards campatibilty issues, but now transitions can also have begin_checkpoint => [['from' => ['shutting_down'], 'to'=>'final_checkpointing'], ['from' => ['ready', 'processing'], 'to' => 'checkpointing']] Test added. 5.3 compatibilty maintained.
RobvH
force-pushed
the
State-plus-transition-keys-destination
branch
from
f4d7719
to
b39a265
Compare
|
@winzou Have you seen this PR? I would greatly appreciate any feedback you have. |
|
@winzou It would be great to have that feature for us as well. Any chance that this would be merged soon? |
|
@winzou See you active about the repo now... It's been over a year without comment from you on a PR I expected would have been merged in a few hours. Do you have any questions or thoughts about this? I'm available to discuss or help out in any way possible. |
|
I still think this is a very useful addition and belongs in every state machine library. It seems it's just a patch not a big rewrite and so does not justify forking off the project and having the burden to maintain the fork afterwards. @winzou So please merge. Though it won't benefit me any longer since I had the luxury to switch to Node.js for my websocket server. :-/ |
| @@ -1,3 +1,4 @@ | |||
| vendor | |||
| bin | |||
| composer.lock | |||
| .idea | |||
There was a problem hiding this comment.
Can you put it in your own .git/info/exclude file?
There was a problem hiding this comment.
Can... generally don't b/c some projects do check this in. 👍
| $transition = $this->config['transitions']['create']; | ||
|
|
||
| $this->config['transitions']['create'] = array( | ||
| $transition |
There was a problem hiding this comment.
Looks like you only test that an array of 1 transition works the same as before.
Could you add a test for the case where a transition name contains multiple transitions defined on different from states? So that we're sure that we call the right transition, depending on the current state.
There was a problem hiding this comment.
👍 great catch, I absolutely will.
| /** | ||
| * @param $transitionName | ||
| * | ||
| * @return bool|mixed |
There was a problem hiding this comment.
null|array maybe?
It's more consistent to return null than false when there is no matching transition.
There was a problem hiding this comment.
👍 agreed; it's more semantic this way. Thanks again. I'll include this in an update later today/tonight.
|
@winzou Will have a look during lunch break today! Thanks!! |
Previously the state machine machine was able to express multiple source states for a transition, but was unable to define the complete source + transition = destination.
Consider processing + begin_checkpoint = checkpointing. However, for the begin_checkpoint transition a different destination state must be defined for another source state. E.g. shutting_down + begin_checkpoint = final_checkpointing, etc.
No backwards campatibilty issues, but now transitions can also look like this:
Test added.
5.3 compatibilty maintained.