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

Add terminal states to TKStateMachine. #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ipmcc
Copy link

@ipmcc ipmcc commented Feb 7, 2014

This patch adds the ability to specify terminal states for the machine. Once the machine transitions into a terminal state it will refuse to fire any more events. Patch also includes assorted other fixes (like eliminating the use of property setters in -init and transitioning off of shouldBeNil/shouldNotBeNil which are deprecated in Kiwi).

/**
Returns a Boolean value that indicates if the receiver has transitioned into one of the states in `terminalStates`.
*/
@property (nonatomic, readonly) BOOL terminated;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be isTerminated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly. I deliberated
this a bit myself, but I felt like if it wasn't just "terminated" that it
should really be "hasTerminated" but that doesn't really go with the naming
convention. I guess terminated isn't a great word... maybe finished would
be better... shrug

@blakewatters
Copy link
Owner

What's the use case on this?

@ipmcc
Copy link
Author

ipmcc commented Feb 7, 2014

I was trying to come up with a way of nesting state machines. Specifically,
I needed a way to tell the parent when the child reached a terminal state.
While I was able to achieve this by setting didEnter blocks on all the
terminating states in the child, it felt awkward, and it kinda ruins the
encapsulation/reuse benefits of the nested state machine, by strongly
coupling the child to the parent. Adding a first class API for "terminated"
gives an abstract/generic way to detect that the state machine has entered
a terminal state without the block coupling the parent and child machines.
(i.e. KVO on the terminated property or using the NSNotification.) I toyed
with the idea of adding a didTerminate block property, but concluded that
just moved the coupling and didn't eliminate it.

@@ -237,9 +253,16 @@
extern NSString *const TKStateMachineIsImmutableException;

/**
A Notification posted when the `terminated` state of a `TKStateMachine` changes to YES
*/
extern NSString *const TKStateMachineDidChangeStateNotification;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be TKStateMachineDidTerminateNotification ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed.

@blakewatters
Copy link
Owner

I did some more thinking on this:

Couldn't the terminalStates property be eliminated by evaluating whether or not there are any exit events from the current state? This would eliminate the need for explicit configuration. We could instead add a method:

- (BOOL)isTerminalState:(TKState *)state;

that introspects the transitions from the state to determine if it is indeed terminal. From there, we can emit the terminated error by evaluating if [self isTerminalState:self.currentState].

I would probably also add the willTerminate and didTerminate blocks to the state machine.

@ipmcc
Copy link
Author

ipmcc commented Feb 7, 2014

This wouldn't work in the face of events with nil for sourceStates which I was using -- that's why I didn't go this way initially.

@blakewatters
Copy link
Owner

I wonder if this doesn't argue for eliminating the support for nil as a value for source state. That would make the implementation more closely model a traditional state machine and is only slightly more configuration, while being much more explicit. I don't love the idea of adding the terminalStates property to support both of these features in parallel.

@blakewatters
Copy link
Owner

I would like to revive this terminal state work for TransitionKit 3.0 by removing support for nil source states and using introspection of the available transitions.

@jhollida24
Copy link

The idea of nested machines is a good one and I would love to revive this conversation, unless there's something easy I'm missing to accomplish that another way.

@blakewatters, is this something you're still considering?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants