A better way to pass transition arguments

[lexuzieel]

Currently it is possible to pass arguments using variable number of arguments since Fire() function is variadic. This works, however this sort of defeats the purpose of static type checking that Go provides while also bloating user code with type checks.

While I have no definite solution in mind, I would like to discuss a better solution for this. Namely, what if we introduced an argument wrapper structure that handled most of typical use cases that users could need.

In my mind I have two specific use-cases:

1. User always passes a single argument, typically a pointer to a complex struct (i.e. chat bot message struct)
2. User passes multiple primitive arguments akin to program start arguments (strings, integers, floats)

We could introduce a stateless.TransitionArguments struct which would be a wrapper for the underlying []interface{}. Then we could tackle both use cases using such wrapper.

First use case

For the first use case we could introduce First() and Empty() methods:

// Check to see if there are any arguments at all
func (ta *stateless.TransitionArguments) Empty() bool
// Return the first argument from the list
func (ta *stateless.TransitionArguments) First() interface{}

Then we could use it like so:

fsm.Configure("state_a").Permit("step", "state_b").
    OnEntry(func(ctx context.Context, args stateless.TransitionArguments) error {
        var arg = args.First() // returns interface{}
        if arg == nil {
            return errors.New("argument must not be nil")
        }

        var message = arg.(*Message) // Which you could later cast (risking segfault if not careful)

    fmt.Printf("Recieved: %s", message)

        return nil
    })

We could further simply it like this:

fsm.Configure("state_a").Permit("step", "state_b").
    OnEntry(func(ctx context.Context, args stateless.TransitionArguments) error {
        if args.Empty() {
            return errors.New("argument must not be nil")
        }

        var message = args.First().(*Message)

    fmt.Printf("Recieved: %s", message)

        return nil
    })

I am not sure, but maybe there is a way to use reflection to enforce static contract for a value to be either of specified type or nil? This could help avoid segfault during runtime.

Second use case

For the second use case we could define a fluent API similar to what genv uses.

fsm.Configure("state_a").Permit("step", "state_b").
    OnEntry(func(ctx context.Context, args stateless.TransitionArguments) error {
        if args.Has(0) != true {
            return errors.New("argument 0 is required")
        }

        var text string = args.At(0).String()
        var count int = args.At(1).Default(1).Integer()

    fmt.Printf("Recieved: %s, %d", text, count)

        return nil
    })

Of course, this will be a breaking change, but I am interested in what is your opinion on this. There still persists a problem that there is no way to enforce contract for Fire() method, but I feel like input validation is much more error-prone and useful in the end.

[qmuntal]

Strongly-typed transition arguments can already be achieved using StateMachine.SetTriggerParameters, see a (probably too short) example here: https://github.com/qmuntal/stateless#parameterised-triggers

If this does not cover your needs I'm open to consider your proposal.

[lexuzieel]

I have read the description of SetTriggerParameters and although it sort of guards against incorrect type (avoiding panicking with a segfault), however it still panics, from which you have to recover somewhere. Is there a preferred way to handle such panics? Maybe before Fire()ing an event?

The way I suggest it allows to check arguments when entering a state, linking all error-handling logic to that state. That would allow to put custom logic along-side argument retrieval (i.e. sending bot message with an error to the user) alongside returning Go error.

If I understand correctly, StateMachine.SetTriggerParameters adds a check that runs when Fire() is called and panics there before moving on?

[qmuntal]

If I understand correctly, StateMachine.SetTriggerParameters adds a check that runs when Fire() is called and panics there before moving on?

Yep.

The way I suggest it allows to check arguments when entering a state, linking all error-handling logic to that state. That would allow to put custom logic along-side argument retrieval (i.e. sending bot message with an error to the user) alongside returning Go error.

Please help me understand the use case. Sending a bot message to the user saying "hey, the programmer did not use the correct type when calling Fire(triggetY), here is his number: XXX" doesn't seem like the right user experience. I would rather have a recover which catch any Fire() panic and sends a generic error to the user.

If you really need to do some state-specific error handling logic around argument types, you can already do so by not using SetTriggerParameters and just casting the interfaces the usual way.