1

I have a set of Writer classes each with a different implementation. I have a list of writers that offers the same interface. Calling a method on the list should invoke the same method on each of the elements in the list (composite design pattern). To avoid the overhead of virtual dispatching the writers are stored in a std::variant, and the list iterates over a vector of variants and uses a std::visit and std::invoke to invoke a method on each of the writers.

Given the following piece of C++17 code as a working example implementation:


#include <cstdio>
#include <variant>
#include <string>
#include <functional>

class Writer1
{
  public:
    void writeX(int i)                      { printf("Writer1::writeX(%d)\n",i); }
    void writeY(int i,const std::string &s) { printf("Writer1::writeY(%d,%s)\n",i,s.c_str()); }
    // ... many more writeXYZ functions
};

class Writer2
{
  public:
    void writeX(int i)                      { printf("Writer2::writeX(%d)\n",i); }
    void writeY(int i,const std::string &s) { printf("Writer2::writeY(%d,%s)\n",i,s.c_str()); }
    // ... many more writeXYZ functions
};

class WriterList
{
    using WriterVariant = std::variant<Writer1,Writer2>;

#define FORALL(name)                                                   \
    template<class... As>                                              \
    void forall_##name(As&&... args)                                   \
    {                                                                  \
      for (auto &v : m_writers)                                        \
      {                                                                \
        std::visit([&](auto &&o) {                                     \
              using T = std::decay_t<decltype(o)>;                     \
              std::invoke(&T::name,o,std::forward<As>(args)...);       \
            },v);                                                      \
      }                                                                \
    }

    FORALL(writeX);
    FORALL(writeY);
    // ... many more writeXYZ functions

  public:
    void add(WriterVariant &&v)             { m_writers.push_back(std::move(v)); }
    void writeX(int i)                      { forall_writeX(i);   }
    void writeY(int i,const std::string &s) { forall_writeY(i,s); }
    // ... many more writeXYZ functions
  private:
    std::vector<WriterVariant> m_writers;
};

int main()
{
  WriterList wl;
  wl.add(Writer1());
  wl.add(Writer2());
  wl.writeX(42);
  wl.writeY(2,"Hello");
}

The FORALL macro is only needed for the member reference &T::name. I would prefer to pass that as a (template) parameter and avoid the ugly macro. It this somehow possible?

Live Demo

cigien
  • 57,834
  • 11
  • 73
  • 112
doxygen
  • 14,341
  • 2
  • 43
  • 37

1 Answers1

1

Would this work for you? I got rid of the macro by using a template class whose methods return the pointer to member:

class WriterList
{
    using WriterVariant = std::variant<Writer1,Writer2>;

    struct tag_writeX{};
    struct tag_writeY{};

    template <class T>
    struct helper
    {
        static auto get(tag_writeX){return &T::writeX;}
        static auto get(tag_writeY){return &T::writeY;}
    };


    template<class Tag, class... As>
    void forall(As&&... args)
    {
        for (auto &v : m_writers)
        {
            std::visit([&](auto &&o) {
                using T = std::decay_t<decltype(o)>;
                std::invoke(helper<T>::get(Tag{}), o, std::forward<As>(args)...);
            }, v);
        }
    }

public:
    void add(WriterVariant &&v)             { m_writers.push_back(std::move(v)); }
    void writeX(int i)                      { forall<tag_writeX>(i);   }
    void writeY(int i,const std::string &s) { forall<tag_writeY>(i,s); }
    // ... many more writeXYZ functions
private:
    std::vector<WriterVariant> m_writers;
};

EDIT: Different approach

Based on your comment, perhaps the below would be more appropriate. I use one helper class for each of your write* methods, but I've removed the ones from your WriterList so there is no replication. The difference now is that there is only one WriterList::write method (templated) which you call by passing the relavant helper class as follows:

namespace Writer
{
    template <class T>
    struct X{static constexpr auto pm = &T::writeX;};

    template <class T>
    struct Y{static constexpr auto pm = &T::writeY;};
}

class WriterList
{
    using WriterVariant = std::variant<Writer1,Writer2>;

public:

    template<template <class> class W, class... As>
    void call(As&&... args)
    {
        for (auto &v : m_writers)
        {
            std::visit([&](auto &&o) {
                using T = std::decay_t<decltype(o)>;
                std::invoke(W<T>::pm, o, std::forward<As>(args)...);
            }, v);
        }
    }

    void add(WriterVariant &&v)             { m_writers.push_back(std::move(v)); }

private:
    std::vector<WriterVariant> m_writers;
};

int main()
{
    WriterList wl;
    wl.add(Writer1());
    wl.add(Writer2());
    wl.call<Writer::X>(42);
    wl.call<Writer::Y>(2,"Hello");
}
linuxfever
  • 3,763
  • 2
  • 19
  • 43
  • It does nicely get rid of the macro, so in that sense it is an improvement and an answer to my question. Ideally, I would like to do without the tags or have these as a (template) parameter, so I have less duplication. – doxygen Mar 05 '22 at 12:10
  • @doxygen I've edited my answer and added another approach based on your comment. Would this work for you? – linuxfever Mar 05 '22 at 14:14
  • 1
    Yes, I like it. Some cosmetic improvements I can think of: move `X` and `Y` into a `Writer` namespace and rename `WriterList::write` to `WriterList::call`. Then in main the calls would look like: `wl.call(42);` which is still very readable without polluting the global namespace unnecessarily. – doxygen Mar 05 '22 at 16:28
  • @doxygen yup, done – linuxfever Mar 05 '22 at 16:43