Enabling multiple triggers for the same state if they have different conditional predicates

[arlm]

This changes follow these rules:

• Throws an InvalidOperationException with the message: "Default triggers and conditional triggers cannot be used on the same resulting state" if the state has triggers with and without a conditions at the same time
• Enters an invalid trigger state if all conditional predates return false

This makes possible to branch a state on multiple conditionals and switch among different final states. This also makes possible to create state machines that embeds some intelligence and also to use triggers as interface component actions with validation.

[prasannavl]

Hi, thanks for the PR. I'll go through this within the weekend. It seems like minor changes, but the I guess the code formatting settings you have used seems to have convoluted the file-diffs, making it difficult to go over the changes.

But I did see a few changes to the representation itself, which changes the internals from List<TriggerRepresentation<TTrigger, TState>>(1); to Dictionary<TriggerRepresentation<TTrigger, TState>, Func<bool>>. I had intentionally used Lists in all the state representations. While dictionaries provide a O(1) lookup, they are expensive in terms of memory for a collection, (and in-fact slower than lists when the items are very small in number). In most general cases, each state is not going to have more than 10 different triggers. While tons of triggers are not uncommon, my intent was to spin off a new optimized class for those needs at some point in the future.

And for small number (5-6) of items, lists will actually be much faster than a dictionary in both lookup, and much cheaper in terms of memory. So, I'm a little skeptical about the change in representations. However, I'll take a detailed look into this soon.

[arlm]

Ohh, sorry for the formatting. Unfortunately my Xamarin Studio settings is to automatically reformat code to the Visual Studio Format due to some projects I use.

Regarding the the List versus Dictionary thing. I do know Dictionaries are a bit "heavier" than Lists but my main intention was not due to performance or the quantity of triggers but enabling to have the same initial state to have the same trigger configured many times with different conditional predicates and different resulting states. I mean some way to introduce branches (like switches or ifs) on the state machine. THis patch would enable this kind of config:

config.ForState(ActivationStates.Paused).OnEntry(() =>
            {
               // some code
            }).OnExit(() =>
            {
                // come code
            }).PermitReentry(ActivationTriggers.Pause)
            .PermitIf(() =>
            {
                return !HasValidEmail && !HasValidSession && !HasValidToken
            }, ActivationTriggers.Online, ActivationStates.Unregistered)
            .PermitIf(() =>
            {
                return HasValidEmail && (!HasValidSession || !HasValidToken);
            }, ActivationTriggers.Online, ActivationStates.InvalidToken)
            .PermitIf(() =>
            {
                return HasValidEmail && HasValidSession && HasValidToken;
            }, ActivationTriggers.Online, ActivationStates.LoggedIn)
            .Permit(ActivationTriggers.Offline, ActivationStates.Offline, () =>
            {
            });

Without my patch (due to using Lists I think but I might be missing something and be completely wrong) the last conditional from ActivationTriggers.Online overwrites the last ones and is the only one registered.

Now thinking better while I analyze what you are saying and meditating over the changes I have done, I think we might be able to achieve this just by changing the FindOrCreate methods like I did to also check for the predicates while searching and/or creating.

Would you like me to try a patch without the dictionary stuff and/or instruct me on the formatting guidelines you use, so that I can enforce it on Xamarin Studio?

[prasannavl]

I'll add the resharper team settings to the project on the next commit for coding conventions. By I'm not sure if Xamarin Studio has any plugins that'll use them. But don't worry about it, as it doesn't do any harm expect that it makes reading diffs slightly harder without a local merge and reformat on my end.

And if you have the time to, I think retaining the lists and just tweaking the FindOrCreate methods to add multiple triggers if predicates are different resulting in different resulting states should do the trick.

And regarding the dictionary or list, my intent is to automatically choose between representations behind the scene, if the number of triggers become large. And also provide it as an option to force the internal representation on the creation factories.

I haven't added it so far because I didn't come across such a large number of triggers scenario yet, and it wasn't possible do it with generics instead of duplicating code. But it should definitely be there at some point :) (There's already a lot of duplicated code, but had to favor them since the JITter will not inline a whole bunch them due to their nature. Hopefully, at some point, Roslyn could help removing the duplicates).

[arlm]

Ohh I see. Your approach would be to be able to have multiple triggers if the resulting state is different. Just note some scenarios that I thought on this.

I really need this feature as I am using your awesome project as the engine to control Android applications using Xamarin and mono to code on C#. I am really glad that your library is completely compatible with mono and Xamarin.

These are the scenarios I thought that may raise concern:

• The user should not have a default (without conditional predicates) trigger if there are other conditional triggers. We might be able to approach it as a valid case if, and only if, when testing on Firing the default case is execute if all other predicates return false;
• There can be multiple predicates on the same trigger given that the user is aware that ideally only one of them is true (not checking this right now) or that the first true predicate encountered is the one used (the approach I opted when implementing. I don't rewrite the rep variable if it is not null, thus the first predicate that returns true get the hold on the trigger).
• If all trigger predicates return false (and the alternative default implementation that I mentioned on the first item is not used) the state is invalid as there are no resulting states when fired.
• It should not never have two default triggers (null predicates) as it is a invalid state (there is no way to ensure what the programmer wanted to be done).

What do you think about my considerations on this? Do they make sense for you? Want me to implement it on one of the ways?

P.S.: I just want to contribute with what I changed for my needs on the best way possible so feel free to ask me to rewrite it on any way you want to lead the project, I willing to adapt it to what you envisioned so that others may use it as I am using.

[prasannavl]

Always happy to accept good contributions. Appreciate it :)

And thanks for the list-down of the behavior. This is precisely the behavior I had in mind. And regarding the second point, your current implementation of the first predicate to be true, is what I had in mind as well. So, its the programmer's job to order the delegates correctly during configuration.

If this is the path to go on, this makes perfect sense. However, I'm not thrilled about the idea of testing multiple delegates. So, I've also been considering a new mechanism instead.

            config.SetInvalidState(State.Invalid);

            config.ForState(State.Connected)
                .OnEntry(() => Console.WriteLine("AOnEntry of Connected"))
                .OnExit(() => Console.WriteLine("AOnExit of Connected"))
                .PermitReentry(Trigger.Connect)
                .AddTransition(Trigger.Talk, () => {
                     Console.WriteLine("Attempting to talk"); 
                     return State.Talking; })
                .AddTransition(Trigger.TurnOff, () => State.Off)
                .AddTransition(Trigger.DoSomethingElse, () => {
                     if (x == 0) return State.Invalid;
                     return State.SomethingElse; });

I'd like to hear your thoughts on this. While the current mechanism is very explicit and readable, this may actually be more concise, and less confusing to program.

There are different approaches how this can be implemented.

1. Use this explicitly for dynamic triggers. Say, call it AddDynamicTransition. And the rest of the machine stays exactly as it is, except that attempted overwrites are no longer allowed, and throws instead to protect programmers from accidentally adding multiple triggers of the same kind. This also means a dynamic transition, and a simple transition cannot co-exist for a given trigger.
2. Replace the entire machine to use AddTransition mechanism instead of the numerous Permit/PermitIf configurations.
3. Use a new DynamicStateMachine, which adds these dynamic functionality as in the first note mentioned above, while the current machines use a flat logic, (i.e, fully remove PermitIf methods, and just add an conditional bool to the Permit predicates, and disable overwrite for that as well. So, for anything with complex logic, one uses the DynamicStateMachine, or for simple machines that can work with flat logic will be able to squeeze a little more performance out.

[arlm]

Wow, this would be great. Better than I envisioned. Of course this is much
more concise and dynamic than my proposal.

I think the way we do today, with my change, leads to a much more static
and defined mechanism. I mean, it permits static checks and things as the
checks for default states and also for simple lambdas (single statement) on
the predicates, although much more difficult to understand if you are not
used to this notation or you are a C# beginner.

I guess your proposal opens to a huge amount of possibilities. I would just
like to know what you thought on it about:

1. I prefer the first option, and would flee from the second. I'm really not sure if it makes sense to have one stateMachine supporting both kinds or it would be best to split, but I have some concerns on adding too much effort for maintenance (I mean now we add DynamicStateMachine, then someone wants it Async also, then someone needs scheduling, etc.)
2. Does this mechanism supports actions after the trigger? Does actions make
   sense on these cases? (I have actions on mine that differ on each case
   preparing for the state change, validating and/or processing information to
   pass to the next state)
3. What if the user/programmer inserts actions inside the if/switch scopes
   inside the predicate, wouldn't it make it more confusing to understand and
   differentiate what happens on each case?

P.S.: Just for your knowledge this patch I submitted is the first step to
a solution that I envisioned. Just wondering and dreaming on higher coding
I though of this for my Android applications: What if my Activity/Fragment
(the window/subpanel of the Android view APIs) also implemented some
interface that also represents a state so that the state machine, when
changing states is really changing views and making actions on my app?
Going a little further, what if we could support substates and these
Activity/Fragments representing states could have internal states (clean,
validated, submitted, loaded, processed, calculated, incomplete, paused,
inBackground, etc) and the buttons/touches represent or fires triggers
either for the substates or for state changes?

[prasannavl]

I believe you followed up on my initial comment. I had revised the comment which already has potential answers to all your questions, except the last.

The API for AddTransition would be AddTransition(TTrigger trigger, Func<TState> resultingState, Action transitionAction);

[prasannavl]

And abstracting Android Views into a giant StateMachine seems like an ambitious undertaking. I'm a little skeptical on accomplishing it in a app-neutral way, but I can truthfully say that I'll be extremely excited to be proved wrong :) But for a customized scenario, why not. But be aware that MVVM ideas are quite close, and on a generic case more appropriate.

[arlm]

I also revised my comment putting numbers.

Regarding my item 1: The possibility of added effort on maintenance due to having a huge quantity of state machines that don't share core among them really concerns me.

My second item is already covered but it leads to item 3. The possibility of someone mixing conditional predicate with actions and leading to a very complex/hard to read and understand code.

Regarding the Android thing. I am now using on two Apps of mine and am thinking on adding this mechanism on a third one. The state machine really controls states on the apps (logged out, filling form 1, filling form 2, processed, offline, online, paused, etc). Of course most states are linked to an Activity and I use the OnEnter to switch among them. The machine is shared among all through a main Activity or the Application class. I have not tried MVVM on Android yet, so yes, it might be an alternative. I just have found out that a number of apps I deal with have workflows that can be directly mapped to states. To these apps it makes really easier to manager using LiquidState so that I can map the allowed transitions and be sure it throws exceptions on unexpected triggers on states or unmapped states. From the ones I work with it was a perfect fit. I guess that it might happen mostly to task specific apps and business oriented apps (i.e. the ones that must follow a strict workflow).

[prasannavl]

PermitDynamic has now been finished.
Pull request over-ridden by commits: ebdf2aa through 3d8b06c

[prasannavl]

Released v6.0.1. Feedback welcome :)