2
\$\begingroup\$

This is the first time that I have tried programming in such a general way using c++. I have tackled this pattern before in other languages such as golang, but this is my first attempt with cpp.

Subscriber template
// class template for a subscriber that wishes to be informed of the
// occurrence of a particular event; the template parameter event type
// is the event of particular interest
template <typename EventType>
struct Subscriber { 
    virtual ~Subscriber() = default;
    // default implementation of the event callback function
    // not sure if it should be a pure virtual (for the compilation error notification)
    // also possibly thinking about changing the signature to return a boolean
    virtual void OnEventReceived(const EventType&) {};
};
Container template
// class template for a subscribers container; the template parameter
// subscriber type is a subscriber of some event type (i.e. some object
// that would like to be notified when some event is triggered)
template <typename SubscriberType>
struct Subscribers {
protected:

    // function template for the _attach method which expects
    // a reference to a subscriber of some event; the subscriber
    // will be added to the pool of subscribers
    template <typename EventSubscriber>
    void _attach(EventSubscriber& s) { 
        subscribers_.emplace_back(
            std::reference_wrapper<EventSubscriber>(s)
        ); 
    }

    // function template for the _detach method which expects
    // a reference to a subscriber of some event; the subscriber
    // will be removed from the pool of subscribers
    template <typename EventSubscriber>
    void _detach(EventSubscriber& s) {
        // erase-remove idiom
        subscribers_.erase(
            std::remove_if(
                subscribers_.begin(),
                subscribers_.end(),

                // mixed feelings about this
                [&s](auto i) { return &i.get() == &s; }
            ),
            subscribers_.end()
        );
    }

    // function template for the _publish method which expects
    // a const reference to an event of some type; the subscribers
    // will be notified of the event trigger via their callback 
    // implementation
    template <typename EventType>
    void _publish(const EventType& e) {
        for (auto s : subscribers_)
            s.get().OnEventReceived(e);
    }

private:
    // using std::reference_wrapper since the subscribers are not owned by
    // the subscribers container, it is merely keeping a pointer to them in
    // order to notify of event occurrences
    std::vector<std::reference_wrapper<SubscriberType>> subscribers_;
};
Publisher template
// class template for an event publisher object; the template parameter
// event types is variadic so that the publisher instance may publish 
// multiple type of events. The multiple inheritance extends the publisher
// to have a subscribers container for the specified event types.
template <typename... EventTypes>
struct Publisher : public Subscribers< Subscriber<EventTypes> >... {

    // function template for the Attach method which expects an event type
    // and a reference to some subscriber object that would like register
    // for notification of the event type in question
    template <typename EventType, typename EventSubscriber>
    void Attach(EventSubscriber &s) { 
        this->Subscribers< Subscriber<EventType> >::_attach(s); 
    }

    // function template for the Detach method which expects an event type
    // and a reference to some subscriber object that would like unregister
    // notifications of the event type in question
    template <typename EventType, typename EventSubscriber>
    void Detach(EventSubscriber &s) { 
        this->Subscribers< Subscriber<EventType> >::_detach(s); 
    }

    // function template for the Publish method which expects a const reference
    // to some type of event that will be broadcast to the currently registered
    // subscribers of the event type in question
    template <typename EventType>
    void Publish(const EventType& e) {
        this->Subscribers< Subscriber<EventType> >::_publish(e); 
    }
};
includes / compilation / questions
#include <vector>
#include <algorithm>
#include <functional>

// compilation
// g++ -std=c++17 main.cpp -o main

// Question 1: 
// should the default implementation of the event callback function stay or
// should be a pure virtual (for the compilation error notification)

// Question 2:
// should attach and detach interfaces be changed to variadic? if so then should
// the recursion take place in Publisher template or the Container template?

// Question 3:
// how to break this up into headers and implementation to maintain a need to
// know basis with other code?

// Question 4:
// thread safety? My assumption is to use mutex in the attach detach methods.
Driver Code

This is just some example usage to see it working not really representative of a particular direction.

#include <iostream>


struct Event1 {
    int x, y;

    void print(std::ostream& os) const {
        os << x << ' ' << y << '\n';
    }
};


struct Event2 {
    int a, b, c;

    void print(std::ostream& os) const {
        os << a << ' ' << b << ' ' << c << '\n';
    }
};


struct SubOne : Subscriber<Event1>, Subscriber<Event2> {

    void OnEventReceived(const Event2& e) override final {
        e.print(std::cout);
    };

    void OnEventReceived(const Event1& e) override final {
        e.print(std::cout);
    };
};


int main(int argc, char** argv) {
    Publisher<Event1, Event2> ePub{};

    SubOne s1{};
    SubOne s2{};

    Event1 e1{2,3};
    Event2 e2{30,21,55};

    ePub.Attach<Event1>(s1);
    ePub.Attach<Event1>(s2);

    ePub.Attach<Event2>(s1);
    ePub.Attach<Event2>(s2);

    ePub.Publish(e1);
    ePub.Publish(e2);

    ePub.Detach<Event1>(s1);
    ePub.Publish(e1);

    return 0;
}

Love to hear any feedback, even outside of the scope the questions section.

\$\endgroup\$
2
\$\begingroup\$

Answers to your questions

Should the default implementation of the event callback function stay or should be a pure virtual?

There is no point in using the base class itself, so it's better to make it pure virtual. This way, bugs in the code where a base class is instantiated by accident are then caught by the compiler.

Should attach and detach interfaces be changed to variadic? If so then should the recursion take place in Publisher template or the Container template?

Since you are asking, the answer is most likely no. If you have no real need for variadic interfaces, I would keep things simple.

How to break this up into headers and implementation to maintain a need to know basis with other code?

It makes sense to have at least a publisher.hpp and a subscriber.hpp, since the various components of your application will need at least one of those two things. As it is now, the code is small enough that I would not split it further.

Thread safety? My assumption is to use mutex in the attach detach methods.

If you do that, you also need to lock the mutex in the publish method, since it will be iterating over the list of subscribers. But yes, if you expect that you will attach, detach and publish from different threads, I would certainly add mutexes.

Code review

The code is quite simple, and if it does exactly what you need, then it's already nice code. I just have these remarks:

Merge the Subscribers class into Publisher

There is no good reason to have a separate class Subscribers, that just takes care of attaching and detaching, and then have Attach() and Detach() methods in Publisher that just wrap the methods from Subscribers.

It would be different if you plan to be able to attach and detach multiple publishers to a given event type. Then you would want to have a separate class that represents the event queue where both publishers and subscribers can connect to.

Avoid names starting with underscores

Names starting with underscores are by convention reserved for the standard library. There are some exceptions, but it is best to just not use such names yourself.

In most cases, it's not necessary at all. Why make a protected function named _attach(), when it is explicitly marked protected in the class definition?

Don't write this-> unnecessarily

It's almost never necessary to write this-> in C++. In the case of Attach() for example, you can just write:

void Attach(EventSubscriber &s) { 
    Subscribers<Subscriber<EventType>>::_attach(s); 
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the feedback, I just wanted to address the explicit use of this->, really to get some clarification. I was reading a blog discussing the use of CRTP, which I don't know if I fully grasped the concept. But it mentioned that the was no guarantees when the template becomes specialized, so using this-> was a mechanism for ensuring functionality. \$\endgroup\$ Mar 27 '20 at 17:53
  • \$\begingroup\$ Hm, I'm not sure about this. Your code seems to compile fine without this->, and I don't think you actually want to allow the code to compile when you create a specialization of Subscribers that doesn't have the attach() member function. But I'm not a language lawyer, maybe someone else can comment on this? \$\endgroup\$
    – G. Sliepen
    Mar 27 '20 at 21:31
  • \$\begingroup\$ Lol language lawyer. I think you are right I didn't realize that the specialization of Subscribers would be the template where the issue would stem from. Yeah there isn't a use case at least that I can see now where one would need to create a specialization of Subscribers. \$\endgroup\$ Mar 27 '20 at 22:07
1
\$\begingroup\$

Code Changes

Following the advice given, I have made the following changes.

Base Subscriber Template

The base callback method has been change to a pure virtual function.

virtual void OnEventReceived(const EventType&) = 0;

Container Template

All the protected methods had the underscore prefix removed from the template function name.

void attach(EventSubscriber& s)  { ... }
void detach(EventSubscriber& s)  { ... }
void publish(const EventType& e) { ... }

Publisher Template

Both the Attach and Detach method interfaces were changed to variadic template parameters and function parameters, and all methods had this-> removed.

template <
    typename    EventType,
    typename... MultiEvent,
    typename    EventSubscriber,
    typename... MultiSubscriber
>
void Attach(EventSubscriber& s, MultiSubscriber&... ms) {
    Subscribers<Subscriber<EventType>>::attach(s);

    if constexpr (sizeof...(ms))
        Attach<EventType>(ms...);

    if constexpr (sizeof...(MultiEvent))
        Attach<MultiEvent...>(s, ms...);
}

template <
    typename    EventType,
    typename... MultiEvent,
    typename    EventSubscriber,
    typename... MultiSubscriber
>
void Detach(EventSubscriber& s, MultiSubscriber&... ms) {
    Subscribers<Subscriber<EventType>>::detach(s);

    if constexpr (sizeof...(ms)) 
        Detach<EventType>(ms...);

    if constexpr (sizeof...(MultiEvent))
        Detach<MultiEvent...>(s, ms...);
}

This opens up a wide range of combinations in the user interface such as:

publisher.Attach<Event1, Event2>(sub1, sub2);
publisher.Detach<Event1, Event2>(sub2);

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.