prasannavl / LiquidState

Efficient asynchronous and synchronous state machines for .NET
Apache License 2.0
240 stars 29 forks source link

PermitIf + Parameterized triggers #26

Open tastyeggs opened 8 years ago

tastyeggs commented 8 years ago

PermitIf must expose the trigger parameter in its predicate for them to be useful. Since config lacks context (as I want to make that a singleton throughout my app), I'm trying to pass in context as a trigger parameter, and would like to add guard conditions that involve this context.

Would suggest an overload such as this:

PermitIf<TArgument>(Func<TArgument,Task<bool>> predicate, ParameterizedTrigger<TTrigger, TArgument> trigger, TState resultingState, Action<Transition<TState, TTrigger>, TArgument> onTriggerAction)

alexmarshall132 commented 8 years ago

I agree. Not being able to pass the argument to the predicate makes no sense and makes using this library much harder and more complicated.

prasannavl commented 8 years ago

This is actually a consequence of a larger problem. The proposed overload such as the above will make for very unclean calls (filled with complicated parameters, and arguments), and will end up in a convoluted mess. This requires an API redesign to have a semantically correct API.

I was waiting for someone to point this out. I didn't want to differ much from the general pattern of Stateless which was the original inspiration when this was created. But I guess it is no longer is relevant to try to keep this similar.

What I had in mind is to use an complete new PermitWith/PermitOn (perhaps even deprecate PermitIf "if that makes sense" in favor of them) function which accepts generic arguments, taking ParameterizedTrigger as the first parameter, so that all the arguments are inferred automatically by the compiler.

This should result in clean code, as well making the intent of arguments very clear.

prasannavl commented 8 years ago

@tastyeggs - This is something that I'd like to work on. However I'm not sure I'll get the time to this in the next few days. So, might take a while.

Meanwhile, I understand there is a need. So, I'll be happy to take in any PRs into the dev branch. Until a full re-design something like what you suggested should atleast help with the task in hand. Unsure if it will make it to the master release, but would be nice to have that code somewhere for people who need it.

alexmarshall132 commented 8 years ago

Thanks Prasanna. That would be great.

On Sat, Feb 27, 2016 at 8:08 PM, Prasanna V. Loganathar < notifications@github.com> wrote:

@tastyeggs https://github.com/tastyeggs - This is something that I'd like to work on. However I'm not sure I'll get the time to this in the next few days. So, might take a while.

Meanwhile, I understand there is a need. So, I'll happy to take in any PRs into the dev branch.

— Reply to this email directly or view it on GitHub https://github.com/prasannavl/LiquidState/issues/26#issuecomment-189772546 .

Alex