boost-ext / sml

C++14 State Machine library
https://boost-ext.github.io/sml
Boost Software License 1.0
1.16k stars 179 forks source link

Guards' access to state data compiles nicely but doesn't follow principle of least surprise #530

Open fiesh opened 2 years ago

fiesh commented 2 years ago

Maybe this has already been brought up -- I couldn't find anything.

Consider this slightly altered example (printIdGuard added as function and guard in transition table, nothing else changd) from the data section:

#include "sml.hpp"
#include <cassert>
#include <iostream>

namespace sml = boost::sml;

namespace {
struct connect {
  int id{};
};
struct disconnect {};
struct interrupt {};

struct Disconnected {};
struct Connected {
  int id{}; /// per state data
};
struct Interrupted {
  int id{}; /// per state data
};

class data {
  using Self = data;

public:
  explicit data(const std::string &address) : address{address} {}

  auto operator()() {
    using namespace boost::sml;

    const auto set = [](const auto &event, Connected &state) {
      state.id = event.id;
    };
    const auto update = [](Connected &src_state, Interrupted &dst_state) {
      dst_state.id = src_state.id;
    };

    const auto printIdGuard = [](auto const &event, Interrupted const &src) {
      std::cout << event.id << ' ' << src.id << '\n';
      return true;
    };

    return make_transition_table(
        *state<Disconnected> + event<connect> / (set, &Self::print) =
            state<Connected>,
        state<Connected> + event<interrupt> / (update, &Self::print) =
            state<Interrupted>,
        state<Interrupted> + event<connect>[printIdGuard] / (set, &Self::print) =
            state<Connected>,
        state<Connected> + event<disconnect> / (&Self::print) = X);
  }

private:
  void print(Connected &state) {
    std::cout << address << ':' << state.id << '\n';
  };

  std::string address{}; /// shared data between states
};
} // namespace

int main() {
  data d{std::string{"127.0.0.1"}};
  sml::sm<data> sm{d, Connected{1}};
  sm.process_event(connect{1024});
  sm.process_event(interrupt{});
  sm.process_event(connect{1025});
  sm.process_event(disconnect{});
  assert(sm.is(sml::X));
}

I'd expect printIdGuard to output 1025 1024, not 1025 0. This really got me pretty good...

Schoppenglas commented 1 year ago

const Interrupted &src vs Interrupted &src

const auto printIdGuard = [](auto const &event, Interrupted &src) {
  std::cout << event.id << ' ' << src.id << '\n';
  return true;
};

Output:

127.0.0.1:1024
127.0.0.1:1024
1025 1024
127.0.0.1:1025
127.0.0.1:1025
fiesh commented 1 year ago

Wow, that's very surprising.