digint/tinyfsm

transit<> shall be the last statement in react(), but isn't ...

taliesin opened this issue · 1 comments

The documentation says:

  1. Implement Actions and Event Reactions

In most cases, event reactions consist of one or more of the following steps:

Change some local data
Send events to other state machines
Transit to different state

Important: Make sure that the transit<>() function call is the last command executed within a reaction function!

However in elevator.cpp:

class Moving
: public Elevator
{
  void react(FloorSensor const & e) override {
    cout << "Reached floor " << e.floor << endl;

    int floor_expected = current_floor + Motor::getDirection();
    if(floor_expected != e.floor)
    {
      cout << "Floor sensor defect (expected " << floor_expected << ", got " << e.floor << ")" << endl;
      transit<Panic>(CallMaintenance);
    }

    current_floor = e.floor;
    if(e.floor == dest_floor)
      transit<Idle>();
  };
};

If the expected floor is not the reported floor it transits to Panic, still sets the current floor to the floor sensor information and might even transit to Idle.

Yeah I know it's more or less cosmetic, but to avoid to confuse newbies (like me) I'd suggest:

class Moving
: public Elevator
{
 void react(FloorSensor const & e) override {
   int floor_expected = current_floor + Motor::getDirection();
   if(floor_expected != e.floor)
   {
     cout << "Floor sensor defect (expected " << floor_expected << ", got " << e.floor << ")" << endl;
     transit<Panic>(CallMaintenance);
   }
  else
  {
     cout << "Reached floor " << e.floor << endl;
     current_floor = e.floor;
     if(e.floor == dest_floor)
       transit<Idle>();
  }
 };
};

Thanks, this was really confusing / plain wrong...

Fixed in: 4cc38dc