virtual void OnEvent(EventT* event) = 0;
void Dispatch(int type, EventT* event);

[/quote]

Aren't these supposed to be
virtual void OnEvent(Event<EventT>* event) = 0;
void Dispatch(int type, Event<EventT>* event);
?

Also, I don't know if somebody has brought this up, but there's a flaw in your system that it seems each type of event is tied to a type of listeners. This means I can't have two different subsystems generating the same type of events but listening to different events.

[font="arial, verdana, tahoma, sans-serif"]

If your compiler supports variadic templates and you are ok to have all message types compiled into your dispatcher you could do something like this (just a demo):

[code removed]

This would add the dependency on variadic templates (I know gcc supports it, but msvc++ 2010 not, for example).
In addition, you have to recompile all modules that depend on SDL_Events if you want to add new messages.
On the other hand, you save the map lookup. It depends on your requirements how much performance you are willing
to sacrifize for modularity.

[/font]
This is an interesting method, but I'm not sure I want to rely on variadic templates just yet. If I'm not mistaken, it's being introduced in the new C++ standard? I would prefer to wait until that becomes standard and more compilers support it. I am using GCC, however, so I will probably mess around with that now and keep it in my toolbox for later as the overhead from the map lookup is one of my biggest issues with my system. Thanks.

Aren't these supposed to be
virtual void OnEvent(Event<EventT>* event) = 0;
void Dispatch(int type, Event<EventT>* event);
?

Not on the first one, unless I wanted to convert the event to type EventT in each OnEvent so the function could access the data of the actual event (EventT) and not just the base class (Event<EventT>). If I did do conversions each time in the OnEvent functions, I would still have to write a separate function for each event type, so I don't see the benefit in changing OnEvent(EventT*) to OnEvent(Event<EventT>*).

On the other hand, I could absolutely change Dispatch(int, EventT*) to Dispatch(int, Event<EventT>*) and cast the event to an EventT once before dispatching it. The trade-off, then, is that the dispatcher can no longer handle events that don't inherit from that class, so it would be an enforced rule that the Dispatcher could only dispatch event types inherited from my class. And if I enforce that, I may as well take out the "int type" part from each function because I could just get that type via Event<EventT>::kID in the function itself. That would simplify the calls from Register<EventT>(type, &listener) and Dispatch(type, &event) to just Register<EventT>(&listener) and Dispatch(&event), which seems more intuitive to me. Interesting.

Also, I don't know if somebody has brought this up, but there's a flaw in your system that it seems each type of event is tied to a type of listeners. This means I can't have two different subsystems generating the same type of events but listening to different events.

I'm not sure I know entirely what you mean. Yes, each event type Event<EventT> is tied to a listener type EventListener_Interface<EventT>, but the classes that inherit from EventListener_Interface<EventT> can handle any number of event types in their own unique ways:

class Listener1 : public EventListener_Interface<KeyDownEvent>, public EventListener_Interface<KeyUpEvent> {
public:
void OnEvent(KeyDownEvent* event) {
// do something unique to this class

}

void OnEvent(KeyUpEvent* event) {
// do something unique to this class
}
};

class Listener2 : public EventListener_Interface<KeyDownEvent> {
public:
void OnEvent(KeyDownEvent* event) {
// do something unique to this class

}
};

int main() {
EventDispatcher disp;
Listener1 l1;
Listener2 l2;

disp.Register<KeyDownEvent>(&l1);
disp.Register<KeyUpEvent>(&l1);
disp.Register<KeyDownEvent>(&l2);

dist.Dispatch(new KeyDownEvent(some, key, data));
}


If I completely missed your point, I apologize.

Personally I dislike the whole Java-esque 'inheriate to recieve messages' setup. I contend that a delegate pattern is far more flexible and probably just as fast.

However as I've already written a series of massively long posts on it recently I shant go down that route again.

Personally I dislike the whole Java-esque 'inheriate to recieve messages' setup. I contend that a delegate pattern is far more flexible and probably just as fast.

However as I've already written a series of massively long posts on it recently I shant go down that route again.

I'm not closed off to the idea of using a delegate pattern, I just haven't seen one done well enough to make me use it. I did just skim over the... uh... book of an argument between you and speciesUnknown on this subject (and bookmarked it for later in-depth reading), but I would appreciate any insights you have on the subject. I'm not looking for an argument, but rather some guidance as, honestly, I just want something that works well, is flexible, and allows me to get the job done.

I'm not looking for an argument, but rather some guidance as, honestly, I just want something that works well, is flexible, and allows me to get the job done.

1) You are given a class, let's say ThirdPartyFoo. You want to notify its bar() when something happens. Suddenly, you are inheriting third-party code that might not have been meant to inherit.

2) The classes are provided to you by a factory, all you get is base interface IBase which has one pure virtual method foo(). Nothing to inherit, you don't even have class definitions

3) You want to call free function. If using SDL, or many C-like libraries, there won't be many classes or methods, so nothing to inherit.

The solution to above is always a wrapper class. Preferably an anonymous class. This works great in Java or C#, even in other languages - because they are memory managed.

In C or C++, you are suddenly stuck with a temporary anonymous object whose lifecycles is a total mess between event system and instance it operates on.

But it gets better. Since the callback definitions are polymorphic by very nature, they cannot be passed around by value. References cannot be stored. So suddenly there's a bunch of pointers, possibly wrapped in smart pointers, making things very complicated and verbose.

---

Delegates are basically boost::function. A value class, passed and copied arbitrarily. They are capable of dispatch to free or member functions, can deal with any type of polymorphism and require minimal bookkeeping.

In short (ignoring signature overhead):struct Event {
void add(boost::function<void()> callback);
void dispatch() {
for_each(events.begin(), events.end(), ...);
}
private:
vector<boost::function<void()>> events;
};And that's it.

All the problems solved by ::function, works with any and every situation possible and takes basically 5 lines to implement.

It's even possible to abandon the Event class and just keep vectors around.

PS: ::function will become standard, there are variadic templates, etc.... The above just works reliably today across all platforms and compilers made over last 10 or more years.

Generally I'm against the function pointers.

Function pointers can be seen as anonymous interface with single function. All interfaces could be done by function pointers. Sure they are flexible, but if it gets out of hand, the program can be very hard to reason about. If some call fails, it might get tricky to pinpoint the target site, with debugger or by searching source code.

This case is a single function interface though. Luckily interface vs function pointer solutions are orthogonal, you can do both.


template<typename EventT>
class EventListener
{
public:
virtual ~EventListener_Interface() {}
virtual void OnEvent(EventT* event) = 0;
};

template<typename EventT>
class FunctionPointerEventListener : public EventListener<EventT>
{
std::function<void(EventT*)> m_callback;

public:
FunctionPointerEventListener(std::function<void(EventT*)> callback)
: m_callback(callback)
{
}

void OnEvent(EventT* event)
{
m_callback(event);
}
};


Generally I'm against the function pointers.

Which works well, until one needs to use third-party libraries.

It's not really a preference, but it simply cannot be done any other way.

All interfaces could be done by function pointers.[/quote]
A typical Java observer interface has 9 methods. With exception of demos, only one of them will ever be used.

See this.

A prime example of redundant bloat. Single function version:void windowCallback(WindowEvent e, ChangeType t);And no, the other version is not more OO, it doesn't help with polymorphic aspects at all. All such code ends up using instanceof, which is worse than a switch that would be required for second version.

Sure they are flexible, but if it gets out of hand, the program can be very hard to reason about.[/quote]I'll counter with Java. Almost no listener is ever implemented beyond one single method. So one is suddenly burdened with abstract helpers and adapters, causing no end to code bloat.

If some call fails, it might get tricky to pinpoint the target site, with debugger or by searching source code.[/quote]
What's the difference between boost::function<void()> failing and ICallback * failing? Both are just pointers.

If using debugger, callstack points out where something failed.


Anyone using Qt however will be wary of function pointers. Because Qt is basically a port of Java idioms and standard library combined with intrinsic VM-like resource management and *all* of C++ stuff. A bloat of epic proportions that makes boost look like Hello World application.

[font="arial, verdana, tahoma, sans-serif"]

This case is a single function interface though. Luckily interface vs function pointer solutions are orthogonal, you can do both.

[/font][font="arial, verdana, tahoma, sans-serif"]I don't know why I would implement both interfaces and function pointers. It seems to me that using function pointers would make the FunctionPointerEventListener unnecessary and would just add an extra layer of unnecessary complexity, as well as the overhead of a virtual function call and function pointer call.[/font]

[color="#1C2837"]If some call fails, it might get tricky to pinpoint the target site, with debugger or by searching source code

How is that different from an interface? Shouldn't the debugger point out where the call failed in both the function pointer and the interface?

[quote name='Ftn' timestamp='1315516949' post='4859218']
Generally I'm against the function pointers.

Which works well, until one needs to use third-party libraries.

It's not really a preference, but it simply cannot be done any other way.
[/quote]
I'm not sure I agree. It can be done using interfaces, but it, again, adds an extra layer of complexity. For instance, if I wanted a function from another library to be called when an event happened I would make a new class that inherits from the base event listener class and calls that function in OnEvent.

All interfaces could be done by function pointers.

A typical Java observer interface has 9 methods. With exception of demos, only one of them will ever be used.

See this.

...

I'll counter with Java. Almost no listener is ever implemented beyond one single method. So one is suddenly burdened with abstract helpers and adapters, causing no end to code bloat.
[/quote]
In my current implementation I only inherit if and when the object is one that needs to be notified when an event of the inherited listener interface type happens. If an object of that class is not registered, however, I guess I would end up with a bunch of unused OnEvent functions...

I am starting to lean toward using a delegate pattern and function pointers for this.

So I converted my system to using function pointers as opposed to interfaces, but the only drawback seems to be that it is a bit slower. I tested each system with 500 listeners and 50000 events and blank member functions for the objects passed. On average, the old interface system dispatched all events to all listeners in ~134 ms while the function pointer system dispatched all events in ~208 ms.

Here's my updated system:
class EventDispatcher {
public:
// typedefs: ===============================================================
typedef std::vector<void*> ListenerVector;
typedef std::map<int, ListenerVector> ListenerMap;

// ctors/dtor: =============================================================
EventDispatcher() {}
virtual ~EventDispatcher() {}

// public functions: =======================================================
// - listeners_ access
template<typename EventT>
void Register(int type, void (*func)(EventT*));

template<typename EventT, typename ClassT>
void Register(int type, void (ClassT::*func)(EventT*), ClassT* obj);

// - dispatch event
template<typename EventT>
void Dispatch(int type, EventT* event);

private:
// vars: ===================================================================
ListenerMap listeners_;
};

template<typename EventT>
void EventDispatcher::Register(int type, void (*func)(EventT*)) {
listeners_[type].push_back(new std::function<void(EventT*)>(func));
}

template<typename EventT, typename ClassT>
void EventDispatcher::Register(int type, void (ClassT::*func)(EventT*), ClassT* obj) {
listeners_[type].push_back(
new std::function<void(EventT*)>(
std::bind(func, obj, std::placeholders::_1)));
}

template<typename EventT>
void EventDispatcher::Dispatch(int type, EventT* event) {
if (event == NULL) { return; } // disallow a NULL event

ListenerMap::iterator itr = listeners_.find(type);
if (itr == listeners_.end()) { return; } // type not found

// dispatch event to the listeners
for (unsigned int x = 0; x < itr->second.size(); ++x) {
// I know, I know, this is fugly, but it didn't appear to affect
// performance to static_cast the void* to std::function* in my tests.
(*static_cast<std::function<void(EventT*)>*>(itr->second.at(x)))(event);
}
}

TestListener/TestEvents:

class TestEvent1 : public inp::Event<TestEvent1> {
public:
TestEvent1(int x, int y, int z) : x_(x), y_(y), z_(z) {}

int x_, y_, z_;
};

class TestEvent2 : public inp::Event<TestEvent2> {
public:
TestEvent2(const std::string& str) : str_(str) {}

std::string str_;
};

class TestListener
: public inp::EventListener_Interface<TestEvent1>,
public inp::EventListener_Interface<TestEvent2> {
public:
TestListener();

void OnEvent(TestEvent1* event);
void OnEvent(TestEvent2* event);
void OnTestEvent1(TestEvent1* event);
};


TestListener::TestListener() {}

void TestListener::OnEvent(TestEvent1* event) {
}

void TestListener::OnEvent(TestEvent2* event) {
}

void TestListener::OnTestEvent1(TestEvent1* event) {
}

And here is the code I used to test it:
int listeners = 500, events = 50000;
lib::Timer timer;

// TEST FUNCTION-BASED EVENTDISPATCHER ---------------------------------------
EventDispatcher func_disp;

timer.Start();
for (int x = 0; x < listeners; ++x) {
func_disp.Register(TestEvent1::kID,
&TestListener::OnTestEvent1,
new TestListener);
}
cout << "fn: " << timer.GetTicks() << "\n";

timer.Start();
for (int x = 0; x < events; ++x) {
func_disp.Dispatch(TestEvent1::kID, new TestEvent1(1, 2, 3));
}
cout << "fn: " << timer.GetTicks() << "\n\n";

// TEST INTERFACE-BASED EVENTDISPATCHER --------------------------------------
inp::EventDispatcher interface_disp;

timer.Start();
for (int x = 0; x < listeners; ++x) {
interface_disp.Register<TestEvent1>(new TestListener);
}
cout << "in: " << timer.GetTicks() << "\n";

timer.Start();
for (int x = 0; x < events; ++x) {
interface_disp.Dispatch(new TestEvent1(1, 2, 3));
}
cout << "in: " << timer.GetTicks() << "\n";

So, is there something that I'm doing wrong for it to be slower than the old system, or is that just the nature of the beast?

Ok, although I don't use std::function too often, I noticed some things to improve:
- you are leaking memory
- why don't you just let users register a std::function object directly?

In addition, you could hide some boilerplate (event id , event base ) with this technique:


unsigned int next_id()
{
unsigned int previous_id = 0;
return ++previous_id; //or return previous_id++ , depending on wheter you need a NULL value
}
template< class Type >
class type_id
{
public:
static unsigned int value;
};
template< class Type >
unsigned int type_id< Type >::value = next_id(); //unique value for each type

After that, the register and disptach functions would just look like this:

template< class EventType >
void register( std::function< void( const EventType &) > callback )
{
unsigned int id = type_id< EventType >::value;
//...
}
template< class EventType >
void dispatch( const EventType &event )
{
unsigned int id = type_id< EventType >::value;
//...
}

That way, you can use any type you want (e.g. an int or an std::string) as an EventType and save a lot of boilerplate and bugs because you can't accidentally use a wrong event id.

To save the map lookup, you could use a vector whose size is as big as the number of EventType s you have instead of a map.
(to do is, check in register and dispatch whether your vector is large enough and resize it if needed).
Then you can just access the listener_vector for any type T with

listener_vectors[ type_id< T >::value ];

which has O(1), but probably consumes more memory if you have multiple dispatchers.