2

I have a class that has multiple constructors:

class Obj
{
    public:
        Obj(int x, int y)           {}
        Obj(std::string const& str) {}
        Obj(char v, int s)          {}
};

Now I want to add an Options object to the constructor that is stored in the object. But to make this work well I want to move the options when I can but copy when I have to. It seems like I have to double up the number of constructors to support both move and copy of the options.

class Options {};

class Obj
{
    Options options;

    public:
        Obj(Options const& o, int x, int y):          options(o) {}
        Obj(Options const& o, std::string const& str):options(o) {}
        Obj(Options const& o, char v, int s)         :options(o) {}

        Obj(Options&& o, int x, int y):          options(std::move(o)) {}
        Obj(Options&& o, std::string const& str):options(std::move(o)) {}
        Obj(Options&& o, char v, int s)         :options(std::move(o)) {}
};

If I had used templates I could have used perfect forwarding and got the correct affect.

template<typename Options>
class Obj
{
    Options options;

    public:
        Obj(Options&& o, int x, int y):          options(std::forward<Options>(o)) {}
        Obj(Options&& o, std::string const& str):options(std::forward<Options>(o)) {}
        Obj(Options&& o, char v, int s)         :options(std::forward<Options>(o)) {}
};

The trouble with this is that I know the type Options.

Martin York
  • 257,169
  • 86
  • 333
  • 562
  • Do you really need perfect forwarding? Just copy it by value should be fast enough (assuming that the cost of a move is negligible). – user202729 Jan 25 '21 at 03:37

2 Answers2

2

Forwarding references only work with templates, you could templatize the constructors and impose restriction on template parameter.

class Obj
{
    Options options;
    template<typename Opt>
    using ValidOption = std::enable_if_t<std::is_same_v<Options, std::decay_t<Opt>>, bool>;


    public:
        template <typename X = Options, ValidOption<X> = true>
        Obj(X&& o, int x, int y):          options(std::forward<Options>(o)) {}
        template <typename X = Options, ValidOption<X> = true>
        Obj(X&& o, std::string const& str):options(std::forward<Options>(o)) {}
        template <typename X = Options, ValidOption<X> = true>
        Obj(X&& o, char v, int s)         :options(std::forward<Options>(o)) {}
};
songyuanyao
  • 169,198
  • 16
  • 310
  • 405
  • That has a down side in usage: `Obj a({}, 1, 2);` Does not work as the compiler can not deduce the type of `{}` were it would work if I had used `Options` directly. This means I shifting the burden from myself onto the user of the class (something I would prefer not to do). – Martin York Jan 25 '21 at 03:31
  • @MartinYork Does `template >* = nullptr>` work? – NathanOliver Jan 25 '21 at 04:00
  • @songyuanyao If you add NathanOliver's trick (and remove the template around the class) that makes it work like I want. Still ugly but works. – Martin York Jan 25 '21 at 04:05
  • BTW, the constraint `std::is_same` is very restrictive, [`std::is_constructible`](https://en.cppreference.com/w/cpp/types/is_constructible) seems more adapted. – Jarod42 Jan 25 '21 at 11:06
1

Your last snippet:

template <typename Options>
class Obj
{
    Options options;

public:
    Obj(Options&& o, int, int):            options(std::forward<Options>(o)) {}
    Obj(Options&& o, std::string const&) : options(std::forward<Options>(o)) {}
    Obj(Options&& o, char, int)          : options(std::forward<Options>(o)) {}
};

is not perfect forwarding, as it is your class which is template, not your function.

Perfect forwarding would be:

class Obj
{
    Options options;

public:

    template <typename T> Obj(T&& o, int, int):            options(std::forward<T>(o)) {}
    template <typename T> Obj(T&& o, std::string const&) : options(std::forward<T>(o)) {}
    template <typename T> Obj(T&& o, char, int)          : options(std::forward<T>(o)) {}
};

I summarize (performance) options in Taking sink parameters by rvalue reference instead of by value to enforce performant usage of interfaces

So unless performance is really important and even an extra move is important, I would go for

Obj(Options o, int x, int y) : options(std::move(o)) {}

or

Obj(Options&& o, int x, int y) : options(std::move(o)) {}

(depending how important you want to avoid implicit copy).

If you target performance, you have indeed to go with forwarding reference.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • I am not so worried about efeciency. I am trying to remove code duplication (or I am experiementing with removing code duplication). I have a library with lots of classes and the `Options` being passed down through multiple layers. Then having to have multiple constructors for objects at each layer. The amount of code duplication (with the only difference being a move or pass by ref) increases exponentially. The code is usually trivial but if I was to come along later and change something small the cascade of changes to push that trough could be huge (and error prone). – Martin York Jan 25 '21 at 22:16
  • So take by value or r-value reference is a good option. – Jarod42 Jan 25 '21 at 22:21
  • What happens if the object is const? Is that going to invoke the value version and force a copy. – Martin York Jan 25 '21 at 22:22
  • If your initial object is const, a copy is needed somewhere. with const lvalue reference constructor, it happens inside the constructor: with by value, it happens at call-site. with rvalue reference, it happens at call site and should be explicit. – Jarod42 Jan 26 '21 at 01:39