Very simple events implementation

Question

I've been thinking about implementing events and wrote some abstract code:

#include <memory>
#include <vector>
#include <iostream>

template<typename Event>
class Dispatcher
{
public:
    class Listener
    {
    public:
        virtual void OnEvent(Event& event, Dispatcher& sender) {};
    };

private:
    typedef std::shared_ptr<Listener> ListenerPtr;
    typedef std::vector<ListenerPtr> Listeners;
    Listeners _listeners;

public:
    void Reg(ListenerPtr listener)
    {
        if( _listeners.end() != std::find(_listeners.begin(), _listeners.end(), listener) )
            return;

        _listeners.push_back(listener);
    }

    void Unreg(ListenerPtr listener)
    {
        Listeners::iterator iter = std::find(_listeners.begin(), _listeners.end(), listener);
        if( _listeners.end() == iter )
            return;

        _listeners.erase(iter);
    }

    void Dispatch(Event& event)
    {
        for( Listeners::iterator iter = _listeners.begin(); iter != _listeners.end(); ++iter )
            (*iter)->OnEvent(event, *this);
    }
};

struct SomeEvent
{
    int someParam;
    SomeEvent(int someParam) : someParam(someParam) {}
};

class SomeDispatcher : public Dispatcher<SomeEvent>
{
};

struct OtherEvent
{
    int otherParam;
    OtherEvent(int otherParam) : otherParam(otherParam) {}
};

class OtherDispatcher :
    public SomeDispatcher, public Dispatcher<OtherEvent>
{
};

class Consumer :
    public Dispatcher<SomeEvent>::Listener,
    public Dispatcher<OtherEvent>::Listener
{
    virtual void OnEvent(SomeEvent& event, Dispatcher<SomeEvent>& sender)
    {
        std::cout << "OnEvent SomeEvent " << event.someParam << std::endl;
    }

    virtual void OnEvent(OtherEvent& event, Dispatcher<OtherEvent>& sender)
    {
        std::cout << "OnEvent OtherEvent " << event.otherParam << std::endl;
    }
};


int main(int argc, char **argv)
{
    OtherDispatcher dispatcher;

    std::shared_ptr<Consumer> consumer(new Consumer());

    dispatcher.Dispatcher<SomeEvent>::Reg( consumer );
    dispatcher.Dispatcher<OtherEvent>::Reg( consumer );

    dispatcher.Dispatcher<SomeEvent>::Dispatch(SomeEvent(1));
    dispatcher.Dispatcher<OtherEvent>::Dispatch(OtherEvent(2));

    dispatcher.Dispatcher<SomeEvent>::Unreg( consumer );

    dispatcher.Dispatcher<SomeEvent>::Dispatch(SomeEvent(3));
    dispatcher.Dispatcher<OtherEvent>::Dispatch(OtherEvent(4));

    return 0;
}

It compiles and works as expected, but... I have to write more code to inherit from Dispatcher and Listener, to register listener and to define more exactly which dispatcher I'm registering to. And this method does not allow registering of custom class method, but only OnEvent().

Apart from this, are there any issues with this code? Is it compatible with different compilers and platforms? Any suggestions?

Answer 1

Listner needs a virtual destructor
and the OnEvent() method should probably be pure virtual:

class Listener
{
public:
    virtual void OnEvent(Event& event, Dispatcher& sender) {};
};

// Try

class Listener
{
    public:
        virtual ~Listener() {}
        virtual void OnEvent(Event& event, Dispatcher& sender) = 0;
};

You need the virtual destructor because delete will more than likely be called via a pointer to the base class (Listener) at some point.

No point in implementing OnEvent() to do nothing if you need sombody to over-ride it.

Personally I would prefer to use boost::ptr_vector<> rather than a std::vector<std::shared_ptr<>>. The boost vector has a couple of advantages (less lcoks required). Also its members are provided via references rather than pointers so it makes it more natural to use with the standard algorithms (not such a big deal in C++11 (with lambdas) but it makes C++03 much easier to use).

typedef std::shared_ptr<Listener> ListenerPtr;
typedef std::vector<ListenerPtr> Listeners;
Listeners _listeners;

// Replace with:

boost::ptr_vector<Listener>  listeners;

Be careful of using '_` on the front of identifiers. The rules are not obvious so best avoided to make sure you don't actually break them. Underscore Rules

Personally not a fan of the following:

   if( _listeners.end() != std::find(_listeners.begin(), _listeners.end(), listener) )

Nothing wrong with it. But reversing the order of the test just makes you pause a second. See Yoda Conditionals.

Prefer to use standard algorithms rather than hand coded loops.

   for( Listeners::iterator iter = _listeners.begin(); iter != _listeners.end(); ++iter )
        (*iter)->OnEvent(event, *this);

   // Try
   std::for_each(listeners.begin(), listeners.end(),
                 std::bind2nd(Listner::OnEvent, event)
                ); // Assume boost::ptr_vector<>

   // Or go all C++11 on us
   std::for_each(std::begin(listeners), std::end(listeners),
                 [&event](std::shared_ptr<Listener>& lis) {lis->OnEvent(event);}
                );

If you want to have listeners that only deal with a specific type of event. You can write your Listner to be a filtering lsitener.

class Listener
{
public:
    virtual void OnEvent(Event& event, Dispatcher& sender) {};
};

Change to:

template<class ForEvent>
class Listener
{
    public:
        ~Listener() {}

        // This is the method called by the dispatcher.
        void eventFilter(OnEvent& event, Dispatcher& sender)
        {
             // dynamic_cast to a pointer type.
             // Result is NULL if not the correct type.
             // If you used a value type then it would throw an exception.
             ForEvent* specialEvent = dynamic_cast<ForEvent*>(&event);
             if (specialEvent != NULL)
             {
                 // Only call the OnEvent for the types we want.
                 // put back to a references (so OnEvent does not need to
                 // check for NULL).
                 this->OnEvent(*specialEvent, sender);
             }
        }

        virtual void OnEvent(ForEvent& event, Dispatcher& sender) = 0;
};

Answer 2

std::find is in header <algorithm>, which is missing.

For me it did not compile without it.

Answer 3

So, I think the following scenario will break the otherwise elegant Dispatcher class:

The consumer removes itself from the Dispatcher within the OnEvent() notification.

Suppose the example were a single shot timer expiration and the consumer wants to remove itself from notifications once the timer it is interested in expires.