I'm working on a code where I can bind events and callbacks to react to those events, the interface looks like this:
void on_close();
struct S
{
void the_app_is_closing();
};
S s;
Events::Register(app::CLOSE, on_close);
Events::Register(app::CLOSE, s, &S::the_app_is_closing);
...
...
if (/* something happens */)
Events::Broadcast(app::CLOSE);
Internally it keeps a container which associates an enum value identifying an event with all the functions expected to react to that event. Those functions are kept into an object which can hold free functions or member functions and feeds the functions through a template function (apply) that forwards the parameters:
class callback
{
struct base {};
template <typename ... params_pack>
struct callable : public base
{
callable(void(*a_function)(params_pack ...)) :
m_call{a_function}
{}
template <typename listener_t>
callable(listener_t &a_listener, void(listener_t:: *a_function)(params_pack ...)) :
m_call{[&a_listener, &a_function](params_pack ... a_argument)
{
(a_listener.*a_function)(a_argument ...);
}}
{}
std::function<void(params_pack ...)> m_call;
};
template <typename ... params_pack>
auto build(void(*a_function)(params_pack ...))
{
return std::make_unique<callable<params_pack ...>>(a_function);
}
template <typename listener_t, typename ... params_pack>
auto build(listener_t &a_listener, void(listener_t:: *a_function)(params_pack ...))
{
return std::make_unique<callable<params_pack ...>>(a_listener, a_function);
}
std::unique_ptr<base> m_function{nullptr};
public:
template <typename function_t>
callback(function_t a_function) :
m_function{build(a_function)}
{}
template <typename listener_t, typename function_t>
callback(listener_t &a_listener, function_t a_function) :
m_function{build(a_listener, a_function)}
{}
template <typename ... params_pack>
void apply(params_pack ... a_argument) const
{
if (auto &call = *static_cast<callable<params_pack ...> *>(m_function.get());
std::is_invocable_v<decltype(call.m_call), params_pack ...>)
{
call.m_call(a_argument ...);
}
}
};
I have an important bug on that apply function that can be reproduced with this code:
void string_parameter(const std::string &s) { std::cout << s << '\n'; }
void long_parameter(long l) { std::cout << l << '\n'; }
int main()
{
callback l(long_parameter);
callback s(string_parameter);
l.apply(123);
s.apply("Test");
return 0;
}
Even if you can call string_parameter directly with a literal string and long_parameter directly with a literal integer, doing the call through callback::apply messes everything up. I know why it is happening:
- I'm
static_castingcallback::callable<const std::string &>tocallback::callable<const char *>. - Then the
callable::m_callwhich underlying type isstd::function<const std::string &>thinks it isstd::function<const char *>. - The
callable::m_callreceives a literal string but is reinterpreted asstd::stringduring thestd::functioncall, creating the mess.
- Same story with
longandint.
The solution would be to save the parameter pack used on construction in order to use it inside apply:
template <typename function_t>
callback(function_t a_function) :
m_function{build(a_function)}
{ PARAMETERS = function_t.parameters } // ???
template <typename listener_t, typename function_t>
callback(listener_t &a_listener, function_t a_function) :
m_function{build(a_listener, a_function)}
{ PARAMETERS = function_t.parameters } // ???
...
...
template <typename ... params_pack>
void apply(params_pack ... a_argument) const
{
// Saved parameters --> vvvvvvvvvvvvvv
if (auto &call = *static_cast<callable<PARAMETERS ...> *>(m_function.get());
std::is_invocable_v<decltype(call.m_call), params_pack ...>)
{
call.m_call(a_argument ...);
}
}
But I don't know if this is even possible. Any advise?
Thanks!
CodePudding user response：
This is your problem:
class callback
it should be
template<class...Args>
class callback
because you have to think about what happens when the types do not match
void string_parameter(const std::string &s) { std::cout << s << '\n'; }
void long_parameter(long l) { std::cout << l << '\n'; }
callback<long> l(long_parameter);
callback<std::string> s(string_parameter);
l.apply(123);
s.apply("Test");
which works flawlessly.
Now you run into the problem of a central enum for all callbacks.
Events::Register(app::CLOSE, on_close);
Events::Register(app::CLOSE, s, &S::the_app_is_closing);
The problem is that all use of app::CLOSE must know what the signature of the callback must be. The code registering it must know, and the code invoking the callback must know.
Your design, however, carefully forgets this fact, and forces type unsafety at both ends. Then you add so,e template code in the middle to ferry types around... which even if it did work, would be work for no good reason.
template<app::event e>
void Events::Register(event_sig<e>* pf);
template<app::event e, class T>
void Events::Register(T* pt, event_mem_sig<T,e>* pf);
template<app::event e, class...Ts>
void Event::Broadcast(Ts&&....ts);
here we have a more sensible API. The event type is compile time value, so we can do type checking, and store the event callbacks in a type safe list.
...
Now, if you have a reasonably bounded number of events (ie, not 1000s of which under 1% are subscribed to), an even simpler solution is to make an event queue an actual object, instead of an enum and traits.
using token=std::shared_ptr<void>;
template<class...Args>
struct broadcaster {
size_t broadcast(Ts...ts)const;
token subscribe(std::function<void(Ts...)>);
void unsafe_subscribe(void(*)(Ts...));
// IMPLEMENTATION
};
now your code becomes
struct Events {
broadcaster<> appClosing;
};
Events g_events;
struct S
{
void the_app_is_closing();
token listening;
};
S s;
s.listening=g_events.appClosing.subscribe(&s, &S::the_app_is_closing);
g_events.appClosing.unsafe_subscribe(on_close);
g_events.appClosing.broadcast();
The types of the arguments are now tied to the appClosing object, so it is checked at both sibscription and at broadcast, conversion is done automatically.
Here each broadcaster maintains its own listener queue (hence bit above about "1000s of event types most unused). Extra work can be done to reduce the queue storage and share it, but that should onlh be done if you need it. And you probably won't.
The enum solution seems like it reduces duplication, but uniform lists of things with non uniform types are often a sign your list shoudln't be uniform.
Members of a struct are a fine way to list non uniform things. Having them be generated from a template means there isn't code writing duplication. And identical signature broadcasters will share binary implementations, somit isn't inefficient.
CodePudding user response：
The answer of @Yakk - Adam Nevraumont already covers how you could get your callback class to work.
The drawback of their approach is that you need to add a new event broadcaster for each event you would want to have.
I'd like to propose a different way to handle the events that allows you to have arbirary events without having to hard-code them into your Events class.
Type-Based Event System
The main idea of this implementation is to have a class for each event you'd want to have that contains the parameters for the given event, e.g.:
struct AppClosingEvent {
const std::string message;
const int exitCode;
};
struct BananaPeeledEvent {
const std::shared_ptr<Banana> banana;
const std::shared_ptr<Person> peeler;
};
// etc...
This would then allow you to use the type of the event struct as a key for your event listeners.
A very simple implementation of this event system could look like this: (ignoring unregistration for now)
class EventBus {
private:
using EventMap = std::multimap<std::type_index, std::function<void(void*)>>;
// Adds an event listener for a specific event
template<class EvtCls, class Callable>
requires std::is_invocable_v<Callable, EvtCls&>
inline void Register(Callable&& callable) {
callbacks.emplace(
typeid(EvtCls),
[cb = std::forward<Callable>(callable)](void* evt) {
cb(*static_cast<EvtCls*>(evt));
}
);
}
// Broadcasts the given event to all registered event listeners
template<class EvtCls>
inline void Broadcast(EvtCls& evt) {
auto [first, last] = callbacks.equal_range(typeid(EvtCls));
for(auto it = first; it != last; it)
(it->second)(&evt);
}
private:
EventMap callbacks;
};
Register()takes a callable object that needs to be invocable with the given event type. Then it type-erases the callable so we can store it as astd::function<void(void*>Broadcast(evt)looks up all event listeners that are registered based on the type of the event object and calls them.
Example Usage would look like this:
EventBus bus;
bus.Register<AppClosingEvent>([](AppClosingEvent& evt) {
std::cout << "App is closing! Message: " << evt.message << std::endl;
});
bus.Register<BananaPeeledEvent>([](BananaPeeledEvent& evt) {
// TODO: Handle banana peeling
});
AppClosingEvent evt{"Shutting down", 0};
bus.Broadcast(evt);
By using the type of the event as the key both Register() and Broadcast() are completely type-safe - it's impossible to register a function with incompatible function arguments.
Additionally the EventBus class doesn't need to know anything about the events it'll handle - adding a new event is as simple as defining a new class with the members you need for your event.
Adding the ability to unregister an event listener
I chose to use a multimap in this case because they guarantee to not invalidate iterators, unless the element the iterator points to itself gets removed from the multimap - which allows us to use a multimap iterator as the registration token for the event handler.
Full implementation: godbolt example
/*
EventBus - allows you to register listeners for arbitrary events via `.Register()`
and then later invoke all registered listeners for an event type with `.Broadcast()`.
Events are passed as lvalues, to allow event handlers to interact with the event, if required.
*/
class EventBus {
private:
using EventMap = std::multimap<std::type_index, std::function<void(void*)>>;
public:
/*
Represents a registered event handler on the EventBus.
Works a lot like std::unique_ptr (it is movable but not copyable)
Will automatically unregister the associated event handler on destruction.
You can call `.disconnect()` to unregister the event handler manually.
*/
class Connection {
private:
friend class EventBus;
// Internal constructor used by EventBus::Register
inline Connection(EventBus& bus, EventMap::iterator it) : bus(&bus), it(it) { }
public:
inline Connection() : bus(nullptr), it() {}
// not copyable
inline Connection(Connection const&) = delete;
inline Connection& operator=(Connection const&) = delete;
// but movable
inline Connection(Connection&& other)
: bus(other.bus), it(other.it) {
other.detach();
}
inline Connection& operator=(Connection&& other) {
if(this != &other) {
disconnect();
bus = other.bus;
it = other.it;
other.detach();
}
return *this;
}
inline ~Connection() {
disconnect();
}
// Allows to manually unregister the associated event handler
inline void disconnect() {
if(bus) {
bus->callbacks.erase(it);
detach();
}
}
// Releases the associated event handler without unregistering
// Warning: After calling this method it becomes impossible to unregister
// the associated event handler.
inline void detach() {
bus = nullptr;
it = {};
}
private:
EventBus* bus;
EventMap::iterator it;
};
// Adds an event listener for a specific event
template<class EvtCls, class Callable>
requires std::is_invocable_v<Callable, EvtCls&>
inline Connection Register(Callable&& callable) {
auto it = callbacks.emplace(
typeid(EvtCls),
[cb = std::forward<Callable>(callable)](void* evt) {
cb(*static_cast<EvtCls*>(evt));
}
);
return { *this, it };
}
// Broadcasts the given event to all registered event listeners
template<class EvtCls>
inline void Broadcast(EvtCls& evt) {
auto [first, last] = callbacks.equal_range(typeid(EvtCls));
for(auto it = first; it != last;)
(it )->second(&evt);
}
private:
EventMap callbacks;
};
With this you can easily register listeners and unregister them later (e.g. if the class they're bound to gets destructed)
Example:
struct DispenseNachosEvent {};
struct DispenseCheeseEvent {};
class NachoMachine {
public:
NachoMachine(EventBus& bus) {
// register using std::bind
nachoEvent = bus.Register<DispenseNachosEvent>(
std::bind(
&NachoMachine::OnDispenseNachos,
this,
std::placeholders::_1
)
);
// register with lambda
cheeseEvent = bus.Register<DispenseCheeseEvent>(
[&](DispenseCheeseEvent& evt) {
OnDispenseCheese(evt);
}
);
}
// Default destructor will automatically
// disconnect both event listeners
private:
void OnDispenseNachos(DispenseNachosEvent&) {
std::cout << "Dispensing Nachos..." << std::endl;
}
void OnDispenseCheese(DispenseCheeseEvent&) {
std::cout << "Dispensing Cheese..." << std::endl;
}
private:
EventBus::Connection nachoEvent;
EventBus::Connection cheeseEvent;
};
Other benefits
- If you want you can also allow the event handlers to modify the event object - e.g. cancel it - which allows you to return state to the piece of code that called
Broadcast()Example:struct CancelableExampleEvent { inline void Cancel() { isCancelled = true; } inline bool IsCancelled() { return isCancelled; } CancelableExampleEvent(std::string message) : message(message) {} const std::string message; private: bool isCancelled = false; }; // Usage: CancelableExampleEvent evt; bus.Broadcast(evt); if(!evt.IsCancelled()) { // TODO: Do something } - Event Handlers can remove themselves - this is usually tricky to implement due to iterators being invalidated, but with multimaps it's rather easy to implement:
By incrementingtemplate<class EvtCls> inline void Broadcast(EvtCls& evt) { auto [first, last] = callbacks.equal_range(typeid(EvtCls)); for(auto it = first; it != last;) (it )->second(&evt); }itbefore calling the function we make sure thatitremains valid, even if the event handler chooses to unregister itself as part of its callback. e.g. this would work:EventBus::Connection con; con = bus.Register<SomeEvent>([&con](SomeEvent&){ std::cout << "Received event once!" << std::endl; con.disconnect(); });
Try it online!
Here's a godbolt that contains the entire code of this post to try it out.