3
\$\begingroup\$

I have a class template that stores an std::optional<T>. A very reduced version of the class template looks like this:

template <typename T>
class param {
public:
    param() = default;
    param(T const& t) : m_value(t) {}

    bool is_valid() const { return m_value.has_value(); }

    T const& value() const
    {
        if (!is_valid()) {
            /* snip */
            assert(is_valid());
        }

        return m_value.value();
    }
private:
    std::optional<T> m_value;
};

Note that the class is meant for lazy evaluation and caching, so in the line containing the comment /* snip */ I omitted a bunch of code irrevelevant to my question. The point of the optional is that a param instance can be in a valid or invalid state and the valid state can be triggered via evaluation.

Now I want the class template to work also with references. This is useful for instance if I want to lazily access a class member.

Since std::optional does not allow the usage of references, I believe I need a partial specialization of param for T&. The partial specialization must store an std::optional<std::reference_wrapper<T>>. But this leads to a lot of code duplicaton, since I really only need to modify the constructor, one line in value and the member definition. Note that here I am showing just a minimal example. In production code, param has much more code, but even there I only need to modify a small percentage of the code in the partial specialization.

template <typename T>
class param<T&> 
{
public:
    param() = default;
    param(T& t) : m_value(std::ref(t)) {}

    bool is_valid() const { return m_value.has_value(); }

    T const& value() const
    {
        if (!is_valid()) {
            /* snip */
            assert(is_valid());
        }

        return m_value.value().get();
    }
private:
    std::optional<std::reference_wrapper<T>> m_value;
};

Live Code

One way to avoid this unnecessary duplication of code is to use inheritence:

template <typename T>
class param<T&> : public param<std::reference_wrapper<T>>
{
    using Base = param<std::reference_wrapper<T>>;
public:
    param(T& t) : Base(std::ref(t)) {}

    T const& value() const {
        return Base::value().get();
    }
};

I explicitly did not add a virtual destructor to param<T>, because I want to avoid vtable lookups. I don't intent to ever use the specialization polymorphically. That being said, the class template is in the public API of the library and experience says that if you give a user the possiblity of doing something wrong, some user will eventually do something wrong. In addition, what happens if I add protected members to the unspecialized class template?

So my questions regarding the above code are

  1. Are there any other (better?) approaches to supporting references and avoiding code duplication?
  2. Are there any pros and cons of the two approaches I listed that I have missed? Which way should I go?

Addendum:

Here is yet another option using if constexpr and std::conditional that does not require specialization at all. But IMHO overall the readibility suffers.

template <typename T>
class param {
public:
    param() = default;

    param(T const& t) : m_value(
        [&](){
            if constexpr (std::is_reference_v<T>){
                return std::ref(t);
            }
            else {
                return t;
            }
        }()
    ) {}

    bool is_valid() const { return m_value.has_value(); }

    T const& value() const
    {
        if (!is_valid()) {
            /* snip */
            assert(is_valid());
        }

        if constexpr (std::is_reference_v<T>) {
            return m_value.value().get();
        } else {
            return m_value.value();
        }
    }
private:
    using CacheType = std::conditional_t<
        !std::is_reference_v<T>, 
        std::optional<T>,
        std::optional<std::reference_wrapper<std::remove_reference_t<T>>>
    >;
    CacheType m_value;
};

Godbolt link

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I would avoid inheritance and go for the implementation in the addendum. However, you can significantly simplify it:

  • It's fine to unconditionally use std::ref() in the constructor. If T is not a reference, then temporarily making a std::reference_wrapper for it will not change how m_value is initialized.
  • Similarly, you can unconditionally return m_value.value() in value(), since if it was a std::reference_wrapper, it will invoke the implicit operator T&() that is equivalent to calling .get().
  • You can hoist std::optional out of CacheType.

In summary, you can rewrite your code as:

template <typename T>
class param {
public:
    param() = default;
    param(T const& t) : m_value(std::ref(t)) {}

    bool is_valid() const { return m_value.has_value(); }

    T const& value() const
    {
        if (!is_valid()) {
            /* snip */
            assert(is_valid());
        }

        return m_value.value();        
    }
private:
    using ValueType = std::conditional_t<
        !std::is_reference_v<T>, 
        T,
        std::reference_wrapper<std::remove_reference<T>>
    >;
    std::optional<ValueType> m_value;
};

The using statement is now the only extra line you had to write to support both values and references.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ I stumbled on an issue with the implicit conversions from reference_wrapper<T> to T` in the constructor of param<T>: If T is not a reference and has a greedy constructor, it will take precedence over the conversion operator of reference_wrapper: godbolt.org/z/oh4xGrErb \$\endgroup\$
    – joergbrech
    Commented Nov 17, 2023 at 20:18

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.