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

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

Closed
taliesin opened this issue Jun 26, 2018 · 1 comment
Closed

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

taliesin opened this issue Jun 26, 2018 · 1 comment

Comments

@taliesin
Copy link

taliesin commented Jun 26, 2018

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>();
  }
 };
};

@taliesin taliesin changed the title trans<> shall be the last statement in react(), but isn't ... transit<> shall be the last statement in react(), but isn't ... Jun 26, 2018
@digint
Copy link
Owner

digint commented Jun 26, 2018

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

Fixed in: 4cc38dc

@digint digint closed this as completed Jun 26, 2018
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

2 participants