0
\$\begingroup\$

Given a group of functions that all take similar arguments, eg.

void a(Item& item, int line, std::string const& message);
bool b(Item& item, int line, float adjustment);

(And for other reasons) It's tempting to wrap these into a parameter-structure:

struct ItemLine
{
    Item& item;
    int const line;
    // note that the const member and the reference both result in a deleted
    // operator=, so this can only be constructed, not assigned.  this is inconvenient
    // and will be fixed below -- but it would be nice if C++ had a concept of
    // "const except in operator=" since even though in practice this is overwriting the
    // storage in-place, logically it is replacing one whole object with another.

    constexpr ItemLine(Item& item_, int line_) noexcept : item(item_), line(line_) {}
};

void a(ItemLine itemLine, std::string const& message);
bool b(ItemLine itemLine, float adjustment);

Buuut, sometimes you want the Item to be const:

int c(Item const& item, int line);

This can be done by making a copy of ItemLine that declares Item const& item instead (and a similar change to the constructor).

This quickly gets unwieldy however if there's more than one ref/pointer type (so you get more possible combinations of cv-qualification). Also if you want to define additional operations inside the struct that would make sense for both the const-qualified and unqualified cases.

"I know, I'll make a template!"

/// ItemLine.hpp
template<typename T>
struct BasicItemLine
{
    // not strictly necessary, due to the explicit instantiation, but doesn't hurt
    static_assert(std::is_same_v<std::remove_const_t<T>, Item>);

    T& item;
    int line;

    constexpr BasicItemLine(T& item_, int line_) noexcept : item(item_), line(line_) {}

    // this is to let the Mutable version be passed to something expecting a Const
    template<typename = std::enable_if_t<!std::is_const_v<T>>>
    constexpr BasicItemLine(BasicItemLine<T const> const& other) noexcept
        : item(other.item), line(other.line) {}

    [[nodiscard]] int Calculate(int foo) const;
};

// these are used both because I don't want to define the body of Calculate in
// the header and to ensure that it can't be used with any unexpected types.
extern template struct BasicItemLine<Item const>;
extern template struct BasicItemLine<Item>;

using ItemLineConst   = BasicItemLine<Item const>;
using ItemLineMutable = BasicItemLine<Item>;

/// ItemLine.cpp
template struct BasicItemLine<Item const>;
template struct BasicItemLine<Item>;

// the bodies have to be defined twice
int ItemLineConst::Calculate(int foo) const { ... }
int ItemLineMutable::Calculate(int foo) const { ... }

This does work, but it's a bit messy, and still doesn't scale very well if there's more than one value you want to select the const-ness for.

I did also consider tuples, which seem a bit tidier (apart from one quirk):

struct ItemLineMutable final : std::tuple<Item*, int>
{
    using tuple::tuple;

    constexpr ItemLineMutable(Item& item_, int line_) : tuple(&item_, line_) {}

    [[nodiscard]] constexpr Item& item() const noexcept { return *std::get<0>(*this); }
    [[nodiscard]] constexpr int line() const noexcept { return std::get<1>(*this); }

    [[nodiscard]] int Calculate(int foo) const;
};
struct ItemLineConst final : std::tuple<Item const*, int>
{
    using tuple::tuple;

    constexpr ItemLineConst(Item const& item_, int line_) : tuple(&item_, line_) {}

    [[nodiscard]] constexpr Item const& item() const noexcept { return *std::get<0>(*this); }
    [[nodiscard]] constexpr int line() const noexcept { return std::get<1>(*this); }

    [[nodiscard]] int Calculate(int foo) const;
};

Here, tuple takes care of the storage, and also (via the inherited constructor) appears to magically take care of the implicit conversion from ItemLineMutable to ItemLineConst. The field access is a little more annoying since it requires using getter methods instead of plain fields (or the abomination that is accessing by index), but presumably the compiler will inline these away to nothing in the end anyway.

It still requires implementing Calculate twice, but now also requires declaring it twice as well (which can get annoying if you want to define many of these). And it's still almost as tricky to scale this to more than one optionally-const type (though this does take better care of the conversions for you, so you're less likely to miss one than doing it manually -- and it's cleaner than using extern templates).

Note that I had to make the tuple hold a pointer rather than a reference, for two reasons. The first is that for some mysterious reason tuple-with-Item& requires Item to be fully defined (not merely a forward reference; due to some internal type trait inspection), which is unacceptable to me. The second is that I wanted operator= to still work. Using reference_wrapper would have been nice (since it does the same thing for much the same reason), but sadly that's not constexpr. (This has the slightly undesired side effect of allowing construction from a pointer as well as a reference, but I consider that relatively harmless (apart from stray nullptrs, but then that's their own fault). It could be fixed by omitting the inherited tuple constructor, but then the mutable-to-const conversion would have to be made explicitly again.)

Is there any way to further improve on this whole concept?

As an aside: constexpr methods make me sad. Ideally the compiler should not require any methods to be declared constexpr or not -- it should figure out at the point of declaring a constexpr variable whether everything involved in its initialisation can be called at compile time or not, without special annotation on the methods (since by definition it must have the full implementation visible or it's an error anyway). The current system leads to methods that would otherwise be completely fine as constexpr being uncallable because someone forgot to annotate it.

\$\endgroup\$
  • 2
    \$\begingroup\$ I'm not the downvoter, but currently your code is borderline hypothetical code because of generic names like Item or a b c. You can make your question on-topic (and also make your code more useful) is to support any type, not just Item, by making the type a template parameter. \$\endgroup\$ – L. F. Feb 14 at 7:50
  • \$\begingroup\$ Is this code that is currently working in a project or are you looking for opinions of what is best? Please see the code review guidelines at codereview.stackexchange.com/help/how-to-ask and codereview.stackexchange.com/help/dont-ask. \$\endgroup\$ – pacmaninbw Feb 14 at 11:34
  • \$\begingroup\$ This is extracted from real code, I've just changed the names to protect the guilty. And it's not sensible to make the Item type a template parameter (albeit it already kinda is) because the whole point is to give it a specific field name and some associated interesting helper methods. If I wanted something purely generic, I could just use tuple. \$\endgroup\$ – Miral Feb 14 at 22:51

Your Answer

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

Browse other questions tagged or ask your own question.