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

Feature: Illegal Transitions #10

Open
sebastianhaberey opened this issue Oct 1, 2021 · 11 comments
Open

Feature: Illegal Transitions #10

sebastianhaberey opened this issue Oct 1, 2021 · 11 comments

Comments

@sebastianhaberey
Copy link

Hi, I just tried out the plugin but I couldn't figure out how to define legal vs illegal transitions. I would say they are one of the main reasons to use a state machine. Did I just oversee the feature? Or if not, maybe there's a reason for their absence?

@renggli
Copy link
Owner

renggli commented Oct 1, 2021

State transitions are initiated programmatically through someState.enter(). If you want to condition this you could wrap such code into a method that performs additional checks?

@sebastianhaberey
Copy link
Author

That's definitely possible. But it would mean having one method per state that checks the pre-conditions by looking at the previous state. That would be pretty generic boilerplate code that looks the same in every project that uses the plugin.

Thats why the other state machine plugin has state transition checks built in:

StateTransition turnOn = light.newStateTransition('turnOn', [isOff], isOn);

Sadly that plugin is not up to date (not null-safe). Anyways this is just a suggestion, I solved my problem by implementing a dumbed-down state machine, so I'm good. Feel free to close this!

@renggli
Copy link
Owner

renggli commented Oct 1, 2021

I am definitely open to expand this library, I just don't quite understand the need. Happy to see what can be done, if you could provide a minimally reproducible example?

@IvoSchols
Copy link

Hi,

I created a guard solution, that allows users to define guards for transitions. Effectively creating legal/illegal transitions.

Guards are implemented in the Guard class, and the conditions in the guard are evaluated before notifying listeners. It is hosted on my forked repo: repo.

Is this something you could see implemented in the main repository? Or did I overlook some functionality?

@renggli
Copy link
Owner

renggli commented Jun 24, 2022

Looks reasonable. What I don't like is the untyped context it introduces Map<String, dynamic> and that a very clear command like stateC.enter()is silently ignored in some cases. I still think it would be easier to track state in typed fields and enforce the guards through a clean API that wraps the StateMachine?

@IvoSchols
Copy link

I have implemented the guard decorator by my best understanding in this branch. However I run into two problems:

  1. Although the wrapper is nice for further expansion, it makes the code more convoluted than necessary.
  2. Because states are initialized with the calling machine, the tests rightfully fail. Please allow me to explain:

The standard machine is wrapped with a guard decorator. When a new state is added to the guard decorator, the underlying standard machine is instructed to add a new state. It creates a new state, and returns it to the decorator for guard initialization.

However, the issue arises when the standard machine creates a new state. In the standard newState method, states are constructed with: this. Initializing state.machine = this, referring to the standard machine and not the guard decorator. States therefore do not lookup the guard decorator to set current but rather use the standard machine.
This can be circumvented in two not so clean ways:

  1. making machine non-final in state and setting it after construction in the guard decorator (cleanest of the two, I think).
  2. passing machine in the newState method

Side-notes:

  • I have moved the guards from state to guard decorator, so that the state object does not host code that might not be used. This is achieved with: Map<State, Map<State, Guard>> guards = {};. Not as clean as I hoped it to be. Do you think that there is a better way?

  • I do not think I totally understand what you meant by: tracking states in typed fields.

  • Additionally I do not see how context can be made typed, without losing the possibility of supporting all data types. I think context should have support for all possible data types, so that guards can be constructed in anyway that is needed.

@renggli
Copy link
Owner

renggli commented Jul 2, 2022

I agree, it is unfortunate that the State cannot be customized/wrapped. What about changing newState, newStartState, newStopState to something like the following?

S newState<S extends State<T>>(T identifier, {S Function(Machine<T>, T)? constructor}) {

With tracking states in typed fields, I mean that I don't like to have Map<String, dynamic>. I would rather create classes with strongly typed fields.

@IvoSchols
Copy link

IvoSchols commented Jul 2, 2022

This breaks the invalid machine check, target.machine != this
Which is used to validate states:

if (target != null && target.machine != this) { throw ArgumentError.value(state, 'state', 'Invalid machine'); }

Changed it to: target.machine is! MachineInterface<T>,. All tests pass except: 'set state by unknown state' in group machine.

So context would accept something like varType. Which would be extended by intType, boolType, stringType etc?

@IvoSchols
Copy link

Hi,

I have integrated the commit into my decorator branch of the fork. However a lot of tests are now failing and my debugger cannot step into the failing tests. So I cannot debug as to why they fail.

I understand that you want to keep the core functionality intact and make the code extensible at the same time, but this commit broke so much that I do not know how to continue.

I just needed the guard and context functionality for my studies, and have now sunken too much time into adapting it to the decorator pattern.

I do not think that a clean implementation of the decorator pattern is easily doable without refactoring. Because the machine is not a single object, but actually two interwoven (machine&states).

@renggli
Copy link
Owner

renggli commented Jul 3, 2022

What about something along ...

import 'package:statemachine/statemachine.dart';

class GuardedMachine<T> extends Machine<T> {
  @override
  GuardedState<T> createState(T identifier) =>
      GuardedState<T>(this, identifier);

  @override
  set current(Object? state) {
    final target = state is State<T>
        ? state
        : state is T
            ? this[state]
            : state == null
                ? null
                : throw ArgumentError.value(state, 'state', 'Invalid state');
    // Should probably provide a template method or event to validate the
    // transition before executing it.
    if (current != null &&
        (current as GuardedState<T>).canExit(target as GuardedState<T>)) {
      throw UnsupportedError('Unable to exit to $target');
    }
    if (target != null &&
        (target as GuardedState<T>).canEnter(current as GuardedState<T>)) {
      throw UnsupportedError('Unable to enter to $target');
    }
    super.current = target;
  }

  @override
  GuardedState<T> newState(T identifier) =>
      super.newState(identifier) as GuardedState<T>;
}

class GuardedState<T> extends State<T> {
  GuardedState(super.machine, super.identifier);

  bool canExit(GuardedState<T>? to) => true;

  bool canEnter(GuardedState<T>? from) => true;
}

The casts are a bit ugly, but not sure how to avoid them ...

@IvoSchols
Copy link

IvoSchols commented Jul 3, 2022

Implemented your suggestion in: fork branch

Tests cases all pass. However they are probably not exhaustive.

Two questions

  • Why are GuardedStates nullable in the canExit and canEnter method?

With tracking states in typed fields, I mean that I don't like to have Map<String, dynamic>. I would rather create classes with strongly typed fields.

  • Does this mean wrapping context in its own class with helper functions to restrict access?

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

No branches or pull requests

3 participants