0

I have created simple callback based event manager and it works, but I have some errors with zero template arguments.

class event_manager
{
public:
    template <typename... T>
    static void register_event(const unsigned long e, std::function<void(T...)> ec)
    {
        events.insert({ e, ec });
    }

    template <typename... T>
    static void fire_event(const unsigned long e, T... args)
    {
        for (auto it : events)
        {
            if (it.first == e)
            {
                boost::any_cast<std::function<void(T...)>>(it.second)(args...);
            }
        }
    }
private:
    static std::unordered_multimap<unsigned int, boost::any> events;
};

And I'm using this code to add callback:

event_manager::register_event<unsigned int>(DVU_EVENT_KEY_PRESSED, [](unsigned int key)
{
    //Works!
});

event_manager::register_event(DVU_EVENT_IDLE, []()
{
    //Could not deduce template argument
});

And the second question: Is it possible to change code to remove <unsigned int>-like template specification?

Example:

event_manager::register_event(DVU_EVENT_KEY_PRESSED, [](unsigned int key){}));
DejaVu
  • 155
  • 10
  • The first one doesn't deduce anything. You explicitly told it what to use. Take out that list and it also won't work. Put an explicit list into the second and it will. – chris Oct 24 '14 at 13:30
  • A lambda is unrelated to `std::function`. Template type deduction tries to find types `T...` such that the type `std::function` becomes the same as the type of the argument (the lambda). But that's impossible since those two are unrelated. – dyp Oct 24 '14 at 13:31
  • @chris, but what I have to put into the second call? It has no template parameters. – DejaVu Oct 24 '14 at 13:40
  • You probably also want to look up `e` in `events`, rather than just iterating over the whole map. Otherwise, no point in having a map... – Barry Oct 24 '14 at 13:46
  • @DejaVu, An empty list. – chris Oct 24 '14 at 13:53
  • @chris, I've tried `event_manager::register_event<>(DVU_EVENT_IDLE, [](){});` and it said the same. – DejaVu Oct 24 '14 at 13:55
  • I don't see a reason for that to not work and the first one to work, but OK, I could just be crazy. – chris Oct 24 '14 at 13:57
  • @chris, the first one causes IntelliSense errors (no function template matches the argument list), but it compiles and works. – DejaVu Oct 24 '14 at 14:00
  • This design is ridiculously unsafe. The types used in `register_event` have to match the types used in `fire_event` exactly -- doing that with type deduction is very fragile. – Yakk - Adam Nevraumont Oct 24 '14 at 14:11

3 Answers3

1

Since a lambda is just a functor with operator(), you could just have an overload that ends up deducing it:

template <typename F>
static void register_event(const unsigned long e, F f) 
{
    register_event(e, f, &F::operator());
}

template <typename F, typename R, typename... T>
static void register_event(const unsigned long e, F& f, 
    R (F::*method)(T...) const) 
{
    std::function<R(T...)> func = f;
    events.insert({ e, func });
}

Maybe either require that R == void or static_assert it or something.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • Wow, it works and solved my problems, but it doesn't work with normal functions. – DejaVu Oct 24 '14 at 14:08
  • @DejaVu Usually this is a bad idea, because C++14 brings in `auto` arguments to lambdas, and this technique fails. – Yakk - Adam Nevraumont Oct 24 '14 at 14:10
  • @Yakk You couldn't use a generic lambda in this context anyway. You need to store it by specific type. – Barry Oct 24 '14 at 14:14
  • @Barry Generic lambdas can be stored in `std::function`s just fine. Its genericity is hidden, but it works just peachy. – Yakk - Adam Nevraumont Oct 24 '14 at 14:21
  • @Yakk Right, but I mean, you have to store it by some type, so you'd have to tell the event manager specifically what type you'd want to store it under. At that point, might as well put it in a `std::function` yourself and just use that overload. – Barry Oct 24 '14 at 14:32
  • Works fine for lambda and classes with one version on operator() but doesn't work with std::bind – VestniK Dec 30 '14 at 10:31
1

Even the first one doesn't compile here as you .

std::function is not an exact match for the lambda, and as you use variadic template you can't specify all type that way (as you specify the first type and compiler may deduce the rest).

A possible workaround is to pass just the func

template <typename Func>
static void register_event(const unsigned long e, Func ec);

and reconstruct the std::function with Func::operator()

Jarod42
  • 203,559
  • 14
  • 181
  • 302
0

Your design is unsafe, as relying on type deduction to produce exactly matching types is fragile, and your cast requires exactly matching types.

Here is a slightly different design:

class event_manager {
public:
  template <typename Signature>
  static void register_event(const unsigned long e, std::function<Signature> ec) {
    events.emplace( e, std::move(ec) );
  }

  template <typename Signature, typename...Ts>
  static void fire_event(const unsigned long e, Ts...&& args) {
    auto r = events.equal_range( e );
    for (auto it = r.first; it != r.second; ++it)
    {
      auto&& f = boost::any_cast<std::function<Signature> const&>(it.second);
      f(std::forward<Ts>(args)...);
    }
  }
private:
  static std::unordered_multimap<unsigned int, boost::any> events;
};

here you pass in a function signature at both ends, like void() or void(int). These types have to match exactly.

I perfect forward the arguments to fire_event to the function I pull out of the map.

I did some other improvements, like properly moving/emplacing and removed some spurious copies.

Deducing the signature of a lambda is a bad idea for a few reasons. First, because C++14 auto lambdas are coming. Second, it means the fact that a lambda or function takes a T const& or a T or whatever "leaks" into how you have to call it (your original implementation required all values be taken by-value).

The signature of a given event now is explicitly listed both where you register it, and where you fire it. If it doesn't match, it should be easier to notice.

I would also be tempted to any_cast to/from a pointer, and assert it is non-null, instead of throwing if you get the signature wrong.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524