Designing an EventHandler in C++ I am writing a simple event handling class in C++, to avoid having delegate registration and calls cluttering the rest of the code. I have a design that seems to do the job, but it has a couple of drawbacks.

So far it looks like this:

template&lt;typename T&gt;
struct IDelegate { virtual void operator ()(const T &amp; args) const = 0; };

template&lt;typename T&gt;
class EventHandler
{
public:
    void Trigger(const T &amp; args) const;
    void AddListener(const IDelegate&lt;T&gt; &amp; delegate);
    void RemoveListener(const IDelegate&lt;T&gt; &amp; delegate);
};


Which in practice would look like this (some code elided for clarity):

class Foo
{
public:
    struct CustomEventArgs { int i; };
    EventHandler&lt;CustomEventArgs&gt; * SomeHandler() const { return m_handler; }

    void SomeMethod()
    {
        // Some code
        CustomEventArgs someArgs = { 42 };
        SomeHandler()-&gt;Trigger(someArgs);
    }

private:
    EventHandler&lt;CustomEventArgs&gt; * m_handler;
};

class Bar : public IDelegate&lt;Foo::CustomEventArgs&gt;
{
public:
    void operator ()(const Foo::CustomEventArgs &amp; args) const
    {
        std::cout &lt;&lt; args.i &lt;&lt; " bottles of beer" &lt;&lt; std::endl;
    }

    void StartListening(const Foo &amp; foo)
    {
        foo.SomeHandler()-&gt;AddListener(*this);
    }
};


This code would basically do what I want, but there are still several things I don't quite like.

EventHandler::Trigger() is public

Anyone can call it, which is a serious flaw. Only the owner should be able to call that method. I can see two ways of ensuring this, but I am not happy with any of them:


Making the EventHandler(s) hidden from outside Foo. This defeats the purpose though, since it would then require to add as many AddListener/RemoveListener methods.
Making Trigger() protected, deriving from EventHandler, then declaring Foo as a friend. This would require to declare a specific class for every single pair of event-handler/class-using-that-event-handler, which is less than ideal.


SomeHandler() returns a pointer

If possible, I think it'd be better to make m_handler an actual member instead of having it somewhere (else) on the heap, and return a reference to it. But I don;t see how to do that without getting plagued with non-const-ness: I want StartListening() to take a const reference.

The question[s]


What would you recommend regarding these two issues?
Do you see other flaws in this design?
Been a while since I've done much in the way of heavy C++ &amp; I'm sure others will weigh in with their own suggestions, but my two cents:


I think the best solution is the first one you rejected: add Foo::{Add,Remove}Listener. Yep, it's more code but (a) it solves both the issues you observe with the code & (b) the implementation of these methods will be trivial. It's a compromise: better encapsulation in exchange for a little more code.

If you're really set on your approach, I'm pretty sure you could work around issue #1 by separating EventHandler into two classes (e.g. EventHandler & EventTrigger) & using friends to expose the dirty details, but IMO that's comparatively ugly. This stuff doesn't deserve that kind of hack.

Along that line of thinking but maybe a little cleaner: have EventTrigger derive from EventHandler (the former providing ::Trigger, the latter ::{Add,Remove}Listener), making m_handler an EventTrigger & having Foo::SomeHandler return a pointer to an EventListener -- effectively "hiding" the Trigger method from the outside world. You're still "leaking" here because you'll need to expose a protected instance variable or method (the list of listeners), but this stuff is simple enough that it shouldn't be completely abhorrent.


Depends what you consider a "flaw" :) Implementors of IDelegate might find the fact the method is const to be annoying (but then you might be making assumptions about implementations of IDelegate that I'm not aware of). Other than that & the issues you observed, things look fine to me.