3
\$\begingroup\$

I built a container that can hold objects of various types, and look up an object convertible to a given type. I wrote something about my intended use-case here:

https://stackoverflow.com/questions/58562173/typesafe-way-to-provide-enriched-dependencies-to-derived-classes-in-c

Basically I want to be able to throw a bunch of dependencies onto a pile and then grab the exact version I want from calling code -- for instance, if I just need an IWidget, I can do

IWidget * widget = dependencies.get<IWidget *>();

but if the underlying widget is actually a FancyGreenWidget then I can do

FancyGreenWidget * widget = dependencies.get<FancyGreenWidget *>();

and if it's not fancy and green then I'll get a helpful runtime error during program initialization rather than a surprise segfault sometime while the program is running.

I'm not really familiar with C++17, but it seemed like I needed a bunch of C++17 features to implement my use-case. I feel like someone who writes modern C++ on a daily basis can probably glance at this and tell me several things I'm doing wrong. In particular I suspect there's a much cleaner way to write the visitor in the get<T>() method.

template<class... Types>
  class VariantContainer
  {
    private:

      std::vector<std::variant<Types...>> m_values;

    public:

      template<class T>
      void add(const T &t)
      {
        static_assert(std::disjunction_v<std::is_same<T, Types>...>);

        m_values.push_back(t);
      }

      template<class T>
      T get()
      {
        for(int i = 0; i < m_values.size(); i++)
        {
          std::optional<T> result = std::visit(
            [](auto&& arg) {
              using Targ = std::decay_t<decltype(arg)>;

              if constexpr(std::is_convertible_v<Targ, T>)
              {
                return std::optional<T>(static_cast<T>(arg));
              }
              else
              {
                return std::optional<T>();
              }
          },
          m_values[i]);

          if(result.has_value()) { return result.value(); }
        }

        std::string message = "Could not find member of type ";
        message += abi::__cxa_demangle(typeid(T).name(), 0, 0, nullptr);

        throw std::runtime_error(message);
      }
  };

Sample usage:

    VariantContainer<Base *, Derived1 *, Derived2 *> v;

    Derived2 d;

    v.add(&d);

    Base * b = v.get<Base *>(); // returns a pointer to d

    Derived1 * d1 = v.get<Derived1 *>(); // ERROR

    Derived2 * d2 = v.get<Derived2 *>(); // returns a pointer to d

I'm also in the market for a more descriptive name for this class if anyone has any ideas.

\$\endgroup\$
  • \$\begingroup\$ Do you only want this solution to work only on inheritance hierarchies (as your examples suggest), or also other types of conversion (e.g. float to double, double to int, or more nefarious ones like this example )? \$\endgroup\$ – hoffmale Oct 26 '19 at 12:23
  • \$\begingroup\$ @hoffmale I guess in my use case it's always going to be pointers getting cast to base types. As for how it might be good to behave when other stuff is put it in, I haven't given it much thought. \$\endgroup\$ – Daniel McLaury Oct 26 '19 at 16:45
2
\$\begingroup\$

You said you implemented

a container that can hold objects of various types, and look up an object convertible to a given type

This idea can be simplified and generalized: we want to

look up an object convertible to a given a type in a sequence of variants

We can focus on the get function and strip everything else. The container does not have to be a std::vector<std::variant> — we can use iterators:

template <typename T, typename ForIt>
T get_convertible(ForIt first, ForIt last);

In the code, you want to find an element that is convertible to T, and throw an exception if such an element is not found. You used std::optional to conditionally return the visited element. This overhead can be eliminated:

template <typename T, typename ForIt>
T get_convertible(ForIt first, ForIt last)
{
    auto it = std::find_if(first, last, [](const auto& variant) {
        return std::visit([](const auto& value) {
            using U = std::decay_t<decltype(value)>;
            return std::is_convertible_v<U, T>;
        }, variant);
    });

    if (it == last) {
        throw std::runtime_error{"..."};
    } else {
        return std::visit([](const auto& value) -> T {
            using U = std::decay_t<decltype(value)>;
            if constexpr (std::is_convertible_v<U, T>)
                return value;
        }, *it);
    }
}

(live demo, includes code to disable return warning)

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Just add assert(0); throw something; and you have the warning suppressed portably. \$\endgroup\$ – Deduplicator Oct 26 '19 at 9:04
  • 1
    \$\begingroup\$ @Deduplicator It is not technically possible to suppress a warning "portably" - the standard allows a compiler to issue any number of diagnostics for a well-formed program ;-) Anyway, what's that assert(0); for? A simple throw 0; seems to suffice. \$\endgroup\$ – L. F. Oct 26 '19 at 9:22
  • 1
    \$\begingroup\$ I'm not a C++ programmer, but the two very similar blocks of std::visit in the same method kind of sticks out. Is it possible to work around that? And how big is the overhead with std::optional you mentioned? \$\endgroup\$ – TomG Oct 26 '19 at 11:45
  • 1
    \$\begingroup\$ @TomG Maybe not. std::visit is pretty much the standard way to access a variant in a type-dependent way. As for the overhead of std::optional ... Well, I don't really know, but probably not negligible. \$\endgroup\$ – L. F. Oct 26 '19 at 13:27
  • \$\begingroup\$ @TomG indeed, I initially added the std::optional specifically because I couldn't see a cleaner way to avoid two very similar calls to std::visit. \$\endgroup\$ – Daniel McLaury Oct 26 '19 at 16:49

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.