Pass by value and move, or two methods [duplicate]

Question

This question already has an answer here:

• Why is value taking setter member functions not recommended in Herb Sutter's CppCon 2014 talk (Back to Basics: Modern C++ Style)? 4 answers

Assume I have the following class, which has a method set_value. Which implementation is better?

class S {
public:
  // a set_value method

private:
  Some_type value;
};

Pass by value, then move

void S::set_value(Some_type value)
{
  this->value = std::move(value);
}

Define two overloaded methods

void S::set_value(const Some_type& value)
{
  this->value = value;
}

void S::set_value(Some_type&& value)
{
  this->value = std::move(value);
}

The first approach requires definition of one method only while the second requires two.

However, the first approach seems to be less efficient:

1. Copy/Move constructor for the parameter depending on the argument passed
2. Move assignment
3. Destructor for the parameter

While for the second approach, only one assignment operation is performed.

1. Copy/Move assignment depending on which overloaded method is called

So, which implementation is better? Or does it matter at all?

And one more question: Is the following code equivalent to the two overloaded methods in the second approach?

template <class T>
void S::set_value(T&& value)
{
  this->value = std::forward<T>(value);
}

Answer 1

The compiler is free to elide (optimise away) the copy even if there would be side effects in doing so. As a result, passing by value and moving the result actually gives you all of the performance benefits of the two-method solution while giving you only one code path to maintain. You should absolutely prefer to pass by value.

here's an example to prove it:

#include <iostream>

struct XYZ {
    XYZ() { std::cout << "constructed" << std::endl; }

    XYZ(const XYZ&) {
        std::cout << "copy constructed" << std::endl;
    }
    XYZ(XYZ&&) noexcept {
        try {
            std::cout << "move constructed" << std::endl;
        }
        catch(...) {

        }
    }

    XYZ& operator=(const XYZ&) {
        std::cout << "assigned" << std::endl;
        return *this;
    }

    XYZ& operator=(XYZ&&) {
        std::cout << "move-assigned" << std::endl;
        return *this;
    }

};

struct holder {
    holder(XYZ xyz) : _xyz(std::move(xyz)) {}

    void set_value(XYZ xyz) { _xyz = std::move(xyz); }
    void set_value_by_const_ref(const XYZ& xyz) { _xyz = xyz; }

    XYZ _xyz;
};
using namespace std;

auto main() -> int
{
    cout << "** create named source for later use **" << endl;
    XYZ xyz2{};

    cout << "\n**initial construction**" << std::endl;
    holder h { XYZ() };

    cout << "\n**set_value()**" << endl;
    h.set_value(XYZ());

    cout << "\n**set_value_by_const_ref() with nameless temporary**" << endl;
    h.set_value_by_const_ref(XYZ());

    cout << "\n**set_value() with named source**" << endl;
    h.set_value(xyz2);

    cout << "\n**set_value_by_const_ref() with named source**" << endl;
    h.set_value_by_const_ref(xyz2);
    return 0;
}

expected output:

** create named source for later use **
constructed

**initial construction**
constructed
move constructed

**set_value()**
constructed
move-assigned

**set_value_by_const_ref() with nameless temporary**
constructed
assigned

**set_value() with named source**
copy constructed
move-assigned

**set_value_by_const_ref() with named source**
assigned

note the absence of any redundant copies in the copy/move versions but the redundant copy-assignment when calling set_value_by_const_ref() with nameless temporary. I note the apparent efficiency gain of the final case. I would argue that (a) it's a corner case in reality and (b) the optimiser can take care of it.

my command line:

c++ -o move -std=c++1y -stdlib=libc++ -O3 move.cpp