1
\$\begingroup\$

I'm trying to develop some kind of in-memory store for my entities. These entities are representations of the devices available on the system. There may be eg. 3 leds, 1 temp sensor, 2 engines etc. Some of the devices can be dynamically discoverable, e.g. they can be connected through RS485, USB etc.

I created two solutions, the first one is a store that can handle all devices (they inherit from common Device type) and when one wants to get access to a device stored in the store has to provide a destination type. When performing device lookup there is some kind of type check, verifying whether the destination type matches to current device's type. I wanted to avoid overhead with dynamic cast co I came with my own solution. And this is a part of code I'm not sure what to think about. It works, but do you see any possible pitfalls? What may go wrong? Also how to compare this solution to dynamic_cast in terms of efficiency and safety? I would like to avoid all overhead with rtti.

The second solution is just a generic device store, created for each device type. It's simple, requires no casting, no rtti. But on the other hand, I have to instantiate it for each device type, which may or may not be considered as an advantage. What do you think?

Compiler explorer link: enter link description here C++ ver: 20 Compiled with GCC 10.2

One update regarding this question: I added this "type-check" in order to prevent user from accesing device and casting it to inappropriate type. Let's say that there are two devices in store:

  • id:1 -> Led
  • id:2 -> TempSensor

Without this check user can just get a device using the device id assigned to temp sensor and cast it to a desired type which can be e.g. Led and then we've got the undefined behavior.

Here is the code:

#include <vector>
#include <memory>
#include <optional>
#include <functional>
#include <atomic>

class Device
{
public:
  virtual ~Device() = default;
  explicit Device(int id)
    : id_{id}
  {}

  int id() { return id_ ; }

private:
  int id_;
};

class Led : public Device
{
public:
  explicit Led(int id) : Device(id) {}
};

class TempSensor : public Device
{
public:
  explicit TempSensor(int id) : Device(id)
  {} 
};


//Solution1
class TypeId
{
public:
  template<class T>
  static size_t get()
  {
    using Tp_t = std::decay_t<T>;

    return get_id<Tp_t>();
  }
private:
  template<class T>
  static size_t get_id()
  {
    static auto id = ids_++;

    return id;
  }
  static inline std::atomic<size_t> ids_{};
};


struct DeviceRegistry
{
  struct Config {
    size_t type_id;
    std::unique_ptr<Device> dev;
  };
public:
  template<class T>
  void add(std::unique_ptr<T> device)
  {
    auto type_id = TypeId::get<T>();
    //check if exists
    devices_.push_back(Config{type_id, std::move(device)});
  } 
  template<class T>
  T& get(int device_id)
  { 
    auto config = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c.dev->id() == device_id;
    });
    if (config == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    }

    auto type_id = TypeId::get<T>();

    if (type_id != config->type_id) {
      throw std::runtime_error("Types mismatch");
    }

    return static_cast<T&>(*(config->dev));
  }

private:
  std::vector<Config> devices_;
};

void solution1()
{
  auto registry = DeviceRegistry{}; 
  registry.add(std::make_unique<Led>(1));
  registry.add(std::make_unique<TempSensor>(2));

  registry.get<Led>(1);
  registry.get<TempSensor>(2);

  // registry.get<Led>(2); throws
  // registry.get<TempSensor>(1); throws
}


//Solution2 
template<class T>
struct DeviceStore
{
public:
  void add(std::unique_ptr<T> device)
  {
    devices_.push_back(std::move(device));
  } 
  T& get(int device_id)
  { 
    auto device = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c->id() == device_id;
    });
    if (device == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    } 

    return *(*device);
  }

private:
  std::vector<std::unique_ptr<T>> devices_;
};

void solution2()
{
  auto led_store = DeviceStore<Led>();
  auto tempsensor_store = DeviceStore<TempSensor>();

  led_store.add(std::make_unique<Led>(1));
  tempsensor_store.add(std::make_unique<TempSensor>(1));

  led_store.get(1);
  tempsensor_store.get(1);
}


int main()
{
  solution1();
  solution2();


  return 1;
}

Here is a benchmark source code from quick-bench. Unfortunately I can't just share the link so you have to copy-paste it. I compared solution1 with solution1 with dynamic_cast. Quick bench says my solution1 is 3.3 times faster.

#include <vector>
#include <memory>
#include <optional>
#include <functional>
#include <atomic>

class Device
{
public:
  virtual ~Device() = default;
  explicit Device(int id)
    : id_{id}
  {}

  int id() { return id_ ; }

private:
  int id_;
};

class Led : public Device
{
public:
  explicit Led(int id) : Device(id) {}
};

class TempSensor : public Device
{
public:
  explicit TempSensor(int id) : Device(id)
  {} 
};


//Solution1
class TypeId
{
public:
  template<class T>
  static size_t get()
  {
    using Tp_t = std::decay_t<T>;

    return get_id<Tp_t>();
  }
private:
  template<class T>
  static size_t get_id()
  {
    static auto id = ids_++;

    return id;
  }
  static inline std::atomic<size_t> ids_{};
};


struct DeviceRegistry
{
  struct Config {
    size_t type_id;
    std::unique_ptr<Device> dev;
  };
public:
  template<class T>
  void add(std::unique_ptr<T> device)
  {
    auto type_id = TypeId::get<T>();
    //check if exists
    devices_.push_back(Config{type_id, std::move(device)});
  } 
  template<class T>
  T& get(int device_id)
  { 
    auto config = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c.dev->id() == device_id;
    });
    if (config == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    }

    auto type_id = TypeId::get<T>();

    if (type_id != config->type_id) {
      throw std::runtime_error("Types mismatch");
    }

    return static_cast<T&>(*(config->dev));
  }

private:
  std::vector<Config> devices_;
};


struct DeviceRegistry2
{
public:
  void add(std::unique_ptr<Device> device)
  {
    devices_.push_back(std::move(device));
  } 
  template<class T>
  T& get(int device_id)
  { 
    auto device = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c->id() == device_id;
    });
    if (device == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    }

    return dynamic_cast<T&>(*(*device));
  }

private:
  std::vector<std::unique_ptr<Device>> devices_;
};

void solution1()
{
  auto registry = DeviceRegistry{}; 
  registry.add(std::make_unique<Led>(1));
  registry.add(std::make_unique<TempSensor>(2));

  for (size_t i = 0; i < 10000; i++) {
    registry.get<Led>(1);
    registry.get<TempSensor>(2);
  }
}

void solution_dynamic_cast()
{
  auto registry = DeviceRegistry2{}; 
  registry.add(std::make_unique<Led>(1));
  registry.add(std::make_unique<TempSensor>(2));

  for (size_t i = 0; i < 10000; i++) {
    registry.get<Led>(1);
    registry.get<TempSensor>(2);
  }
}

static void AtomicTypeId(benchmark::State& state) {
  // Code inside this loop is measured repeatedly
  for (auto _ : state) {
    solution1();
  }
}
// Register the function as a benchmark
BENCHMARK(AtomicTypeId);

static void DynamicCast(benchmark::State& state) {
  // Code before the loop is not measured
  for (auto _ : state) {
    solution_dynamic_cast();
  }
}
BENCHMARK(DynamicCast);
\$\endgroup\$
2
  • \$\begingroup\$ Reinterpret cast is not a way to go about this as it gives me no assurance that the destination type matches to type stored in the container. If I wanted to be sure I would have to use dynamic_cast for that purpose but it uses rtti, so I came up with my solution which does not use rtti but gives some kind of "type safety". Btw. without checking whether types match or not using static_cast instead of reinterpret_cast would be sufficient as in this case destination and "stored" types are compatible through the base. \$\endgroup\$
    – bielu000
    May 4, 2022 at 19:11
  • \$\begingroup\$ Sorry for confusing reinterpret_cast with dynamic. My point was to understand the reasons dynamic cast was not chosen. IIRC it does pointer comparison, which is almost the same as your type_id variable. \$\endgroup\$ May 4, 2022 at 20:10

1 Answer 1

2
\$\begingroup\$

I usually do a line-by-line review of the code, but that seems pointless here, because you have three different implementations of the same “thing”, and are ultimately only interested in one. There’s no point in reviewing code in detail when 2⁄3 of it will just go to waste. So instead I’ll do a high-level design review. I’ll just say that the code is generally quite good, and the designs are not bad either.

You have, in essence, three different design strategies:

  1. The “proper” object-oriented C++ way of storing a set of heterogeneous but related types, using a base class and dynamic dispatching. This is what you call “solution 1 with dynamic_cast”.
  2. Your own re-implementation of the above. This is “solution 1”.
  3. Different containers for different types, albeit with a common interface. This is “solution 2”.

I’ll discuss each of these three in order, then add some other suggestions.

Existing designs

Solution 1 with dynamic_cast

This is basically this:

class Device
{
public:
  virtual ~Device() = default;
  explicit Device(int id)
    : id_{id}
  {}

  int id() { return id_ ; }

private:
  int id_;
};


struct DeviceRegistry2
{
public:
  void add(std::unique_ptr<Device> device)
  {
    devices_.push_back(std::move(device));
  } 
  template<class T>
  T& get(int device_id)
  { 
    auto device = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c->id() == device_id;
    });
    if (device == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    }

    return dynamic_cast<T&>(*(*device));
  }

private:
  std::vector<std::unique_ptr<Device>> devices_;
};

Code-wise there’s not much to say; this is more-or-less the “canonical” way to do something like this.

The question I would ask is… why? Why do you want to put different “things” in the same container/registry? Especially considering that you’re just treating them all different in the end anyway.

In other words, if solution 2 makes sense—if it makes sense to put different types of devices in different containers—then why go through all the (costly) gymnastics of jamming them all into a single container?

Normally when you are putting heterogeneous objects in a container like this, it’s because you intend to treat them all the same. For example, with the classic example of a base class of animal and then concrete classes for dog, cat, etc., even though there are different “things” in the container, you’re treating them all the same way:

class animal
{
public:
    virtual ~animal() = default;

    virtual auto feed() -> void = 0;

    virtual auto get_number_of_legs() -> int = 0;
};

class dog : public animal
{
public:
    auto feed() -> void override;

    auto get_number_of_legs() -> int override { return 4; }
};

// ... and so on ...


// You have a container of different types of animals:
auto animals = std::vector<std::unique_ptr<animal>>{};

// You can add different types of animals:
animals.push_back(std::make_unique<dog>{});
animals.push_back(std::make_unique<cat>{});
animals.push_back(std::make_unique<fish>{});

// HOWEVER...
//
// When you *use* this collection, you are treating them all just as
// basic animals.
std::ranges::for_each(animals, [](auto&& animal) { animal->feed(); });

// or:
auto total_number_of_legs = 0;
for (auto legs : animals | std::views::transform([](auto&& a) { return a->get_number_of_legs(); }))
    total_number_of_legs += legs;

You don’t seem to be interested in treating all devices as devices. You seem intent on treating LEDs as LEDs, temperature sensors as temperature senors, and so on. And that makes sense; there aren’t a lot of operations that will be common to all those device types. “Turn on/off” makes sense for LEDs, but not temperature sensors; “get reading” makes sense for temperature sensors, but not LEDs; “set speed” makes sense for a motor, but not LEDs or temperature sensors.

Since you’re not treating them all as devices, forcing them all into a container of devices seems counterproductive. You haven’t provided any explanation for why you want them all in the same container.

So, without more information, I can’t see why this design makes sense.

Solution 1

This is basically this:

class Device
{
public:
  virtual ~Device() = default;
  explicit Device(int id)
    : id_{id}
  {}

  int id() { return id_ ; }

private:
  int id_;
};


class TypeId
{
public:
  template<class T>
  static size_t get()
  {
    using Tp_t = std::decay_t<T>;

    return get_id<Tp_t>();
  }
private:
  template<class T>
  static size_t get_id()
  {
    static auto id = ids_++;

    return id;
  }
  static inline std::atomic<size_t> ids_{};
};


struct DeviceRegistry
{
  struct Config {
    size_t type_id;
    std::unique_ptr<Device> dev;
  };
public:
  template<class T>
  void add(std::unique_ptr<T> device)
  {
    auto type_id = TypeId::get<T>();
    //check if exists
    devices_.push_back(Config{type_id, std::move(device)});
  } 
  template<class T>
  T& get(int device_id)
  { 
    auto config = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c.dev->id() == device_id;
    });
    if (config == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    }

    auto type_id = TypeId::get<T>();

    if (type_id != config->type_id) {
      throw std::runtime_error("Types mismatch");
    }

    return static_cast<T&>(*(config->dev));
  }

private:
  std::vector<Config> devices_;
};

Given the previous design made no sense, this re-implementation also makes no sense.

As for the actual code… it looks mostly fine to me. There are a few things I’d fix:

  1. Device::id() should be const, noexcept, final, and, maybe, constexpr.
  2. Generally you need more const correctness, more noexcept when things can’t fail, and more constexpr wherever possible.
  3. It’s std::size_t, not size_t.
  4. I would suggest std::remove_cvref_t rather than std::decay_t. Actually, I would suggest neither, because this T isn’t deduced, it’s explicitly specified by the user. You should respect what they say. (If they say something stupid, that’s on them.)
  5. TypeId could be simplified quite a bit—no need for the helper function—and made more robust:
template <typename T>
concept device = std::is_base_of_v<Device, T>
    and ((not std::is_const_v<T>) and (not std::is_volatile_v<T>));

class TypeId
{
public:
    template <device T>
    static auto get() noexcept -> std::size_t
    {
        static auto id = ++_next_id;

        return id;
    }

private:
    static inline std::atomic<std::size_t> _next_id = {};
};

The same concept could be used to constrain the device registry functions.

Solution 2

This is basically this:

class Device
{
public:
  virtual ~Device() = default;
  explicit Device(int id)
    : id_{id}
  {}

  int id() { return id_ ; }

private:
  int id_;
};


template<class T>
struct DeviceStore
{
public:
  void add(std::unique_ptr<T> device)
  {
    devices_.push_back(std::move(device));
  } 
  T& get(int device_id)
  { 
    auto device = std::find_if(devices_.begin(), devices_.end(), [&device_id](const auto& c) {
      return c->id() == device_id;
    });
    if (device == devices_.end()) {
      throw std::runtime_error("Cannot find device");
    } 

    return *(*device);
  }

private:
  std::vector<std::unique_ptr<T>> devices_;
};

All the code review issues mentioned before (like no const correctness) still apply.

Design-wise, I would say this design is far superior to solution 1 (either version). I know it appears to be less ergonomic, because you need a separate device store for each type of device, but that’s just because you’re not taking advantage of a number of convenience features.

For example: variable templates. Rather than manually creating a device store for each type of device, and then having to keep track of them all, you can just do this:

template <device D>
class device_store_t
{
public:
    constexpr auto add(std::unique_ptr<D> d)
    {
        _devices.push_back(std::move(d));
    }

    constexpr auto get(int id) const -> D&
    {
        if (auto it = std::ranges::find_if(_devices, [id](auto&& d) { return d->id() == id; }); it != _devices.end())
            return *(*it);
        else
            throw std::runtime_error{"cannot find device"};
    }

private:
    std::vector<std::unique_ptr<D>> _devices;
};

template <device D>
inline constexpr auto device_store = device_store_t<D>{};

// Usage ///////////////////////////////////////////////////////
device_store<led>.add(std::make_unique<led>(1));
device_store<temp_sensor>.add(std::make_unique<temp_sensor>(2));

device_store<led>.get(1);
device_store<temp_sensor>.get(2);

// Alternate usage, with helper function ///////////////////////
template <device D, template... Args>
constexpr auto register_device(Args&&... args)
{
    device_store<D>.add(std::make_unique<D>(std::forward<Args>(args)...));
}

register_device<led>(1);
register_device<temp_sensor>(2);

device_store<led>.get(1);
device_store<temp_sensor>.get(2);

Indeed, there are a ton of API options you could look into. For example, you could register a device with register_device<Type>(args...) and retrieve one with devices<Type>[id].

Unless you have a reason why you want to shoehorn all your devices in a single collection, this is one option that will be both simpler, and more efficient than any solution involving dynamic dispatch.

Other design suggestions

Speaking of dynamic dispatch, all of your existing designs use dynamically allocated device objects, and the registries are containers of (unique) pointers. Sometimes this makes sense, and is even (sorta) necessary, such as in both versions of solution 1, where you are actually using dynamic dispatch. But, especially in solution 2, where each device type gets its own registry, that’s pointless. And even if you really do need to put all devices in a single registry, it’s still possible to avoid the massive costs of dynamic allocation.

Multiple registries with variable templates

Let’s start with improving solution 2.

Once you embrace solution 2—multiple registries, one for each device type—then there is no more need for dynamic allocation to squeeze everything into a single container. You can get enormous performance boots by ditching all the unique pointers, and just storing the devices directly in their registries.

// No more need for a base class, but we can use a concept instead:
template <typename T>
concept device =
    std::is_object_v<T>                                             // no references or functions
    and (not std::is_pointer_v<T>)                                  // no pointers
    and ((not std::is_const_v<T>) and (not std::is_volatile_v<T>))  // no const/volatile
    and requires(T t)                                               // need an id() function
    {
        { t.id() } noexcept -> std::same_as<int>;
    };

// Example device:
class led
{
public:
    constexpr explicit led(int id) noexcept : _id{id} {}

    // moveable
    constexpr led(led&&) noexcept = default;
    constexpr auto operator=(led&&) noexcept -> led& = default;

    // non-copyable
    led(led const&) = delete;
    auto operator=(led const&) -> led& = delete;

    constexpr ~led() noexcept = default;

    constexpr auto id() const noexcept { return _id; }

private:
    int _id;
};

template <device D>
class device_store_t
{
public:
    constexpr auto add(D d)
    {
        _devices.push_back(std::move(d));
    }

    constexpr auto get(int id) const -> D&
    {
        if (auto it = std::ranges::find_if(_devices, [id](auto&& d) { return d.id() == id; }); it != _devices.end())
            return *it;
        else
            throw std::runtime_error{"cannot find device"};
    }

private:
    std::vector<D> _devices;
};

template <device D>
inline constexpr auto device_store = device_store_t<D>{};

// Usage ///////////////////////////////////////////////////////
device_store<led>.add(led{1});
device_store<temp_sensor>.add(temp_sensor{2});

device_store<led>.get(1);
device_store<temp_sensor>.get(2);

// Alternate usage, with helper function ///////////////////////
template <device D, template... Args>
constexpr auto register_device(Args&&... args)
{
    device_store<D>.add(D(std::forward<Args>(args)...));
}

register_device<led>(1);
register_device<temp_sensor>(2);

device_store<led>.get(1);
device_store<temp_sensor>.get(2);

Notice the actual registry is a std::vector<D>… not std::vector<std::unique_ptr<D>>. And devices are created directly, sometimes with perfect forwarding, rather than using std::make_unique() or anything else like that.

Just doing this will get you a huge improvement in speed. And, most importantly, the main benefits will all come where you probably want them the most: when you’re trying to access devices, rather than when you’re registering them.

Single registry with variant type

Okay, but let’s assume you really need all the devices to be stored in a single registry. It’s still possible to do that without any dynamic allocation.

The key is std::variant. If you know all the possible device types that your program will ever use, you can create a variant of them all, and make a registry that holds that variant type:

// Again, no need for a base class, but a concept would still be handy.
template <typename T>
concept device =
    std::is_object_v<T>                                             // no references or functions
    and (not std::is_pointer_v<T>)                                  // no pointers
    and ((not std::is_const_v<T>) and (not std::is_volatile_v<T>))  // no const/volatile
    and requires(T t)                                               // need an id() function
    {
        { t.id() } noexcept -> std::same_as<int>;
    };

// Example device:
class led
{
public:
    constexpr explicit led(int id) noexcept : _id{id} {}

    // moveable
    constexpr led(led&&) noexcept = default;
    constexpr auto operator=(led&&) noexcept -> led& = default;

    // non-copyable
    led(led const&) = delete;
    auto operator=(led const&) -> led& = delete;

    constexpr ~led() noexcept = default;

    constexpr auto id() const noexcept { return _id; }

private:
    int _id;
};

class device_registry_t
{
public:
    template <device D, template... Args>
    constexpr auto add(Args&&... args)
    {
        _devices.emplace_back(std::in_place_type<D>, std::forward(args)...);
    }

    template <device D>
    constexpr auto get(int id) -> D&
    {
        if (auto it = std::ranges::find_if(_devices, [id](auto&& d) { return std::visit([](auto&& dev) { return dev.id(); }, d) == id; }); it != _devices.end())
            return std::get<D>(*it);
        else
            throw std::runtime_error{"cannot find device"};
    }

private:
    std::vector<std::variant<led, temp_sensor, /* any other device types */>> _devices;
};

inline auto device_registry = device_registry_t{};

This won’t be as efficient as having a separate registry for each type, for a couple of reasons:

  1. The registry vector will be larger (because everything will be in one vector, rather than having a separate vector for each type), which will require larger allocations and more reallocations as you add devices. However, this cost is only paid when registering, which, I assume, won’t be that common compared to accessing devices.
  2. Each entry in the registry vector will take up more memory, because each one will not only be the size of the largest device, it will also have to include the variant discriminant (which is probably an int or std::size_t, basically). That means it will be less cache friendly to search through it, so there will be a cost to access.
  3. And, of course, you have to pay for the extra cost of checking that the type is right (which happens in std::get()) every time you access a device.

So this will be quite a bit slower than having a separate registry for each device type… but it will still be much faster than using std::unique_ptrs everywhere. If you really need all your devices in a single registry, then this would be the way to go. If you don’t, I’d recommend the previous solution.

Design comparison and other suggestions

There is very little difference in the ergonomics between all these designs, once you use features like variable templates, perfect forwarding, and so on. In all cases, registration can be as simple as:

register_device<Type>(args...);

And accessing a device can be as simple as:

auto&& dev = get_device<Type>(id);

Or you could have an interface like:

devices.add<Type>(args...);

auto&& dev = devices.get<Type>(id);

You have options, basically.

Assuming you don’t need any more interface than that, there’s no difference between the designs. However, if you need other stuff, like maybe you need to enumerate all devices, or search through them for something, then the designs that put all the different device types into a single registry are better. They will cost you more, of course, but if you need the power, you need to pay the price for it.

One thing I would suggest is maybe don’t make the device ID part of the device object. I obviously don’t know your intended use cases, or your problem domain, but often an identifier for something isn’t a property of the thing itself. For example, consider an employee ID: I have an employee ID at the place I work, but it’s not part of me; it’s not something I carry around as part of my identity, it’s something externally assigned to identify me by the company. In fact, my employee ID can change without me changing in any way at all. I can have multiple employee IDs, too, for different companies, or even within the same company if I do different jobs. And, in theory, my company could assign an employee ID to something that isn’t even a person, like assigning an employee ID to a subcontractor, which would make it possible for that subcontractor to send different people to do their work on different days, and the company neither needs to know or care; they simply pay the employee ID, and the subcontractor sorts out who actually gets the money.

The reason I’m suggesting this is because it will simplify a lot of things. First, it would no longer require every device to truck around an ID it never needs to know or use. If you think about it, why would an LED need to know what ID you are referring to it by? The only properties an LED has are things like colour, whether it’s on or off… properties intrinsic to the LED itself. If devices don’t need to know their own ID, then that simplifies every device class… which is a lot of savings in terms of code, testing, and so on.

Another benefit of devices not having to know their own ID is that it eliminates a whole host of potential errors. For example, this:

auto led_a = led{1};
auto led_b = led{1};

Is that two LEDs? Or is that one LED with two aliases? Is that even legal?

What about this:

auto dev_1 = led{1};
auto dev_2 = temp_sensor{1};

All of those potential problems go away if you remove the responsibility of keeping track of IDs from the devices, and instead make it part of the registry:

class device_registry
{
public:
    template <device D, typename... Args>
    auto add(int id, Args&&... args)
    {
        if (_devices.count(id) != 0)
            throw std::runtime_error{"duplicate ID"};

        _devices.emplace(std::piecewise_construct,
            std::tuple{id},
            std::forward_as_tuple(std::forward<Args>(args)...));
    }

    template <device D>
    auto get(int id) -> D&
    {
        auto&& dev = _devices.at(id);
    
        // do whatever to make sure the type is D
    }

private:
    std::unordered_map<int, device> _devices;
};

If you don’t need to specify the ID, then you could simply use the index in the registry for the device ID. That makes checking whether an ID is valid trivial, and fast:

class device_registry
{
public:
    template <device D, typename... Args>
    auto add(Args&&... args)
    {
        _devices.emplace(std::forward<Args>(args)...);

        return _devices.size() - 1;
    }

    template <device D>
    auto get(std::vector<device>::size_type id) -> D&
    {
        auto&& dev = _devices.at(id);

        // do whatever to make sure the type is D
    }

private:
    std::vector<device> _devices;
};

Of course, if the device ID is something the device itself has to know—like, if it’s the actual pin number or port number or whatever—then it makes sense for it to be part of the device object itself. In that case, you’ll just have to deal with the complexity of making sure device IDs aren’t duplicated, invalid IDs aren’t used, and so on. It all depends on what your use case is.

I benchmarked the five designs, in each case just registering two devices. You’ll see far more divergence in the designs if you register more, of course, but I think this makes the point.

(Oh, as an aside, when you use Quick Bench, and you run a benchmark, and it succeeds, the URL in the address bar changes. You can copy that URL to share the benchmark.)

\$\endgroup\$
15
  • 1
    \$\begingroup\$ “Correct me if I am wrong, but I thought that when you have an interface and several different implementations (like in my case sysfsld, spi led, etc) then it's the perfect place to use dynamic dispatch.” That thinking is decades out of date. That was one of the prevailing philosophies when Java was developed, for example, and even Java eventually adopted generics. Nowadays the problem would usually be solved with templates, which would be simpler, more efficient, easier to verify, and safer. \$\endgroup\$
    – indi
    May 22, 2022 at 17:58
  • 1
    \$\begingroup\$ The problem with run-time polymorphism is that while C++ “supports” it, it isn’t the natural way of doing things. Run-time polymorphism requires pointers or references, but C++ is a value-oriented language; it has pointers and references, but they require more syntax, and have limitations that values do not. Also, generally, run-time polymorphism inhibits or impairs just about every feature of modern hardware that makes it fast: branch prediction, speculative execution, cache coherency, concurrency… etc.. Plus static analysis becomes difficult or impossible, which may be a deal breaker. \$\endgroup\$
    – indi
    May 22, 2022 at 17:58
  • 1
    \$\begingroup\$ Unfortunately, I don’t know of a single resource that covers all of the ways you can avoid run-time polymorphism… because there are so very, very many techniques. If you want to research the field, you could start with key phrases like “static polymorphism” or “compile-time polymorphism”, “generic programming”, “duck typing”, “tagged unions” or “sum types” or “variants” and the “visitor pattern”, and so on—there are just way too many to list. \$\endgroup\$
    – indi
    May 22, 2022 at 17:59
  • 1
    \$\begingroup\$ Let me be clear though that I am NOT saying NEVER use run-time polymorphism. Sometimes it’s the best solution; sometimes it’s the only solution. I’m just saying it should never be your default solution. In my projects, if someone commits something using run-time polymorphism, I would expect them to be able to give a damn good justification for why. If they can justify it, then it’s fine… but “there’s a common interface” would absolutely not be justification enough. \$\endgroup\$
    – indi
    May 22, 2022 at 17:59
  • 1
    \$\begingroup\$ “Global” means more than one thing, so do you mean literally global (in the global namespace), or just logically “global” (accessible from anywhere)? Variable templates can go anywhere that both variables and templates are allowed; they don’t need to be in the global namespace. You could put them in a user namespace, or as a class static data member. But as for logically global… don’t you want a globally-accessible registry? You say “I have access to it pretty much from everywhere” as if that’s a problem. Is it? Do you want it to be impossible to access devices from certain places? \$\endgroup\$
    – indi
    Jun 21, 2022 at 22:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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