patterncppMinor
C++14 Event System
Viewed 0 times
systemeventstackoverflow
Problem
I've created a simplified event system for C++14. The code is commented, so it shouldn't be hard to read. There's also a simple usage scenario below.
It is still a work in progress and uses some not-so-best practices. For example any consumer, that has an access to the event can fire it. Also you will notice, that the subscribers (event handlers) are put into an
Any ideas and critique is welcome.
```
#ifndef __PD_EVENT__
#define __PD_EVENT__
#define PD_EVENT_VER 8
#include
#include
#include
#include
using namespace std::placeholders;
//
// @brief Describes additional parameters for handler binding.
//
enum class EventFlag {
DEFAULT = 0,
ONLY_UNIQUE = 1
};
//
// @version 8
// @brief Defines means for delegate function subscription and calling on demand.
// Maximum allowed number of event arguments is 4.
// @param
//
template
class Event{
//
// @brief Address representation for use as identifier in container mapping.
//
using Address = uint64_t;
//
// @brief Function object with user-defined variadic template arguments.
//
using Handler = std::function;
//
// @brief Type that provides a unique memory-based integral definition for given objects.
// Concept: should be collision-free.
//
using Identifier = uint64_t;
//
// @brief Maximum supported event handler arguments.
//
static constexpr auto _MAX_EVENT_ARGS = 4;
static_assert(sizeof...(Args) event.bind(&MyClass::member, myClassInstance).
//
template
void bind(C (M::member)(), T instance, EventFlag flag = EventFlag::DEFAULT) {
_addToList(_identify(instance, member), std::bind(membe
It is still a work in progress and uses some not-so-best practices. For example any consumer, that has an access to the event can fire it. Also you will notice, that the subscribers (event handlers) are put into an
unordered_map, so they need a unique key, by which we could later determine, whether such a handler already exists. I get this key by 'unionizing' the function object with a uint64_t, multiplying by 10 and then by adding the instance address. I doubt it is a very good idea and platform independent. Any ideas and critique is welcome.
```
#ifndef __PD_EVENT__
#define __PD_EVENT__
#define PD_EVENT_VER 8
#include
#include
#include
#include
using namespace std::placeholders;
//
// @brief Describes additional parameters for handler binding.
//
enum class EventFlag {
DEFAULT = 0,
ONLY_UNIQUE = 1
};
//
// @version 8
// @brief Defines means for delegate function subscription and calling on demand.
// Maximum allowed number of event arguments is 4.
// @param
//
template
class Event{
//
// @brief Address representation for use as identifier in container mapping.
//
using Address = uint64_t;
//
// @brief Function object with user-defined variadic template arguments.
//
using Handler = std::function;
//
// @brief Type that provides a unique memory-based integral definition for given objects.
// Concept: should be collision-free.
//
using Identifier = uint64_t;
//
// @brief Maximum supported event handler arguments.
//
static constexpr auto _MAX_EVENT_ARGS = 4;
static_assert(sizeof...(Args) event.bind(&MyClass::member, myClassInstance).
//
template
void bind(C (M::member)(), T instance, EventFlag flag = EventFlag::DEFAULT) {
_addToList(_identify(instance, member), std::bind(membe
Solution
Reserved Identifiers
Unless you are very sure of the rules about what combinations of leading underscore are reserved for the implementation I would suggest that you do not use underscore (
In particular underscore followed by another underscore or a capital letter is reserved for use by the implementation. For example
Infinite recursion
You system is susceptible to infinite recursion (and crash by stack overflow). Although any small example I can construct will feel contrived and pathological, when using an event system in a complex UI these kind of problems do crop up.
Consider the following (contrived) example:
You could argue that this is invalid use of the system in the same way as
I believe that you should implement some kind of checking to make sure that recursion into an already executing event handler is caught and reported early. Or possibly just stopped and turned into a no-op. In the above example if the recursion into the current caller just immediately returned the behaviour would be correct.
Dangling pointers
Another problem with the system is that it is susceptible to dangling pointers. Consider the following (again contrived) example:
Now of course the fault here is that you forgot to call
One way to do this is to make
Another approach is to use something similar to
Collisions in the Identity
The function to calculate an unique identifier is flawed and is riddled with collisions:
I read the above as:
Closing remarks
I think that you can get rid of your template overloads by clever use of a better identification scheme and using variadic template arguments. However I will not go into detail on this due to lack of time.
Unless you are very sure of the rules about what combinations of leading underscore are reserved for the implementation I would suggest that you do not use underscore (
_) as a prefix. Please see this excellent answer for what identifiers are reserved.In particular underscore followed by another underscore or a capital letter is reserved for use by the implementation. For example
_MAX_EVENT_ARGS is in violation of this rule.Infinite recursion
You system is susceptible to infinite recursion (and crash by stack overflow). Although any small example I can construct will feel contrived and pathological, when using an event system in a complex UI these kind of problems do crop up.
Consider the following (contrived) example:
class CurrencyConverter{
public:
Event euroChanged;
Event dollarChanged;
int euro;
int dollar;
void onEuroChanged(int v){
dollar = convert2dollar(v);
dollarChanged(dollar);
}
void onDollarChanged(int v){
euro = convert2euro(v);
euroChanged(dollar);
}
}You could argue that this is invalid use of the system in the same way as
void f(){f();} is an improper use of recursion. The difference here is that in an event system this kind of recursive behaviour is not always as apparent as it is here, and it may involve a multiply deep call chain from several different objects.I believe that you should implement some kind of checking to make sure that recursion into an already executing event handler is caught and reported early. Or possibly just stopped and turned into a no-op. In the above example if the recursion into the current caller just immediately returned the behaviour would be correct.
Dangling pointers
Another problem with the system is that it is susceptible to dangling pointers. Consider the following (again contrived) example:
Event x;
{
MyClass m;
x.bind(&MyClass::onX, &m);
}
x.fire(...); // Ka-blamo!Now of course the fault here is that you forgot to call
unbind but explicit managing of bind/unbind takes us back to the age of raw new/delete with all the associated problems. I really believe that you need to RAII-fy this system to avoid this kind of bugs.One way to do this is to make
bind to take std::shared_ptr instead of T* and then change subscribers to store , Handler>. Of course this has the drawback that you cannot bind to anything that isn't a shared pointer and this may be unacceptable.Another approach is to use something similar to
std::lock_guard but instead have bind return a EventGuard like so: EventGuard bind(...) that will unbind when it destructs. This is not foolproof either but it shows a few ideas for how you could approach this.Collisions in the Identity
The function to calculate an unique identifier is flawed and is riddled with collisions:
inline static Identifier _identify(C _class, M _member) {
return AddressCast(_class).address * 10 + AddressCast(_member).address;
}I read the above as:
i = a 10 + m. Lets call the lowest address that the system can assign to user heap or stack for a_min, conversely call the largest address a_max. The same goes for m with m_min and m_max. The only way the above expression can be safe is if a_min10 > m_max and a_min10 > a_max. I fired up my debugger and grabed the first heap pointer I could find: a_min a_min10 !> m_max which means that you have collisions in your identity function. This is asking for trouble...Closing remarks
I think that you can get rid of your template overloads by clever use of a better identification scheme and using variadic template arguments. However I will not go into detail on this due to lack of time.
Code Snippets
class CurrencyConverter{
public:
Event<int> euroChanged;
Event<int> dollarChanged;
int euro;
int dollar;
void onEuroChanged(int v){
dollar = convert2dollar(v);
dollarChanged(dollar);
}
void onDollarChanged(int v){
euro = convert2euro(v);
euroChanged(dollar);
}
}Event<X> x;
{
MyClass m;
x.bind(&MyClass::onX, &m);
}
x.fire(...); // Ka-blamo!inline static Identifier _identify(C _class, M _member) {
return AddressCast<C>(_class).address * 10 + AddressCast<M>(_member).address;
}Context
StackExchange Code Review Q#118628, answer score: 4
Revisions (0)
No revisions yet.