Event system with use of function<void (boost::any)> good idea?

Question

I have done a module system, something like this:

//setting event
module->set_event("started", [](boost::any ev) {
  cout << "The module have been successfully started" << endl;
});

//somewhere else
module->start();

//impl
void Module::start() {
  //run once protection here
  this->trigger_event("start");//pre start
  this->_impl->start(); //on error, throw exception
  this->trigger_event("started"); //post start
}

void Module::trigger_event(string str,boost::any ev = boost::any() )
{
  //mutex
  //invokes every handler which have been invoked set_event
}

Really simple and easy to understand. The first drawback is there is no "remove_event", but to allow it (if I need it) can I just return an std::function (in set_event method) which on invocation removes the handler, so if the user want to remove the event, just invoke the returned handler:

function<void ()> h = module->set_event(...);
//... somewhere later
h() //removes the handler safely

But I am an curious if this is a good design? Or will I suddenly discover very large disadvantages using mine approach and will be need to rewrite most code? I can say the "trigger_event" method will also be used as an message system between modules, just like the JavaScript library jQuery method: "bind" allows. An example:

module->set_event("message", [](boost::any message) {
 //deal with the message here
});

//send message
module->trigger_event("message", MyMessageTypeHere);

Is this an accepted design in most cases? I want an very flexible approach therefore is my idea to use boost::any, but is this a good idea(scalable, performance, flexibility) in general?

Answer 1

There are several things that I would change.

First, let's ignore the return for a bit and focus on that boost::any mixed with that std::string. That's really a bad idea for an event system, because it allows a potential runtime type mismatch. You just don't want a design that makes it possible to make those kinds of mistakes (because then those mistakes will be made, eventually, and there will be bugs that need to be fixed, wasting future time).

Typically, what you really want is:

void Module::trigger_event(shared_ptr<Event> const& event)
{
    // Event is a base interface
    // pull off event->ID() to get identifier and lookup
    // dispatch to something that has the signature
    //   void (shared_ptr<SpecificEvent>)
}

To do something like the dispatcher, you usually have

map<IDType, shared_ptr<Dispatcher>> dispatchDictionary_;

with

template <typename SpecificEvent>
class SpecificDispatcher : public Dispatcher
{
public:
    SpecificDispatcher(function<void (shared_ptr<SpecificEvent>)> handler)
        handler_(handler)
    {}

    virtual void dispatch(shared_ptr<Event> event)
    {
        auto specificEvent(static_ptr_cast<SpecificEvent>(event));

        handler_(specificEvent);
    }
};

(with the interface class defined in the obvious way and register/unregister methods to associate the event type IDs in the map). The point is to associate the ID with the EventType inside the class, to ensure it is always the same association, and to dispatch in a way that the handlers do not need to reinterpret the data themselves (error prone). Do the work that can be automated inside your library, so it won't be done wrong outside.

Okay, now what do you return with your method? Return an object, not a function. If they don't need to call it explicitly, you have saved another potential source of error. It's like all RAII - you don't want people to have to remember to call delete, or unlock, or ... Similarly, with "unregister_event" (or whatever you call it).

There are four lifetimes people must be concerned with in a program: expression, function-scope, state, and program. These correspond to the four types of things they can store that object you return in: anonymous(let it fall on the floor - it can be used inside the expression but is never assigned to a named object), an automatic scope object, an object that is a member of a State class (in the State pattern) that exists between two asynchronous transition events, or a global-scope object sitting around until exit.

All lifetime management should be managed with RAII. It's kind of the point of destructors and is how resource cleanup is supposed to be managed in the face of exceptions.

---

EDIT: I kinda left a bunch of the pieces unstated, pointing out you would connect the dots "in the obvious way". But I thought I'd fill in more pieces (as I have an OS build and install going and my bugs are down to one last one right now, which is waiting for the install...)

The point was that someone should be able to just type

callbackLifetimeObject = module->set_event<StartEvent>([](shared_ptr<StartEvent> event){
    cout << "Module started: " << event->someInfo() << endl;
});

so the set_event needs to have the kind of signature to take this, and it should insert the appropriate dispatcher into the dictionary. There are several ways to get the ID from the type here. The "obvious" way is to just create a temporary and call it's "ID" member - but that has object creation overhead. Another way is to turn it into a "virtual static", and just get the static (which is all the virtual method does as well). Whenever I have virtual statics, I tend to turn them into traits - same thing, but slightly better encapsulation, and the ability to modify "already closed classes" as well. Then the virtual method just calls the traits class as well to return the same thing.

So...

template <typename EventType>
struct EventTrait
{
    // typedef your IDType from whatever - looks like you want std::string
    static IDType eventID()
    { /* default impl */ }
};

template <>
struct EventTrait<StartEvent>
{
    // same as above
    static IDType eventID()
    { return "Start"; }
};

then you can

template <typename EventType>
EventRegistration set_event(function<void (shared_ptr<EventType>)> handler)
{
    auto id(EventTrait<EventType>::eventID());

    dispatchDictionary_.insert(make_pair(id, 
        make_shared<SpecificDispatcher<EventType>>(handler)));

    return EventRegistration(bind(& Module::unset_event, this, id));
}

That should illustrate a little better how to fit the pieces together and some of the options you have. Some of this illustrates boilerplate code that may get recoded with each event. That is the type of thing you will likely want to automate as well. There are plenty of advanced techniques to metaprogram such generation, from working with another build step that takes a specification to working inside the c++ metaprogramming system. Again, as with the earlier points, the more you can automate, the less bugs you will have.

Answer 2

It problematic for two reasons.

First of all, you limit your events to handlers that receive exactly one parameter, while judging from your usecase you could have various flavours of events. So, this is limiting (yes you could throw an std::tuple into the any type, to encode several parameters, but do you really want this?).

More importantly, weakening type-safety entails, well, a weakened (type-) safety. If you're event handler can only work with a goat, don't give it a lamb. What you want, seems to be possible to do with templates (in C++11, at least), as you can make trigger_event a type function, that receives an arbitrary number of arguments. It then tries to invoke the handler with this arguments, which will give a compilation error, it you didn't provide the correct number and type of arguments.

Here, in your dispatcher, you can hide the type resolution, which is a bit tricky, because you want to store events of arbitrary type. This could be the place to use any, but kept inside your scheduler implementation/logic, not leaking to your users.

Edit: To achieve this resolution, my approach would be to have some internal abstract Event type (which you can store pointers of) and some template <typename Hnd> classs ConcreteEvent : public Event that allows you to keep your customers Event handler as ConcreteEvent<EventType> (assuming EventType is the template parameter to your set_event method). With rtti you can later check if when retrieving it (you will only retrieve it as Event*) it matches the type you want to call.