1

If I want to define a method like a setter for a std::vector member (or movable object) I can do it like this:

 void set(const std::vector<int>& v) { _v = v; }

But also I need to add

 void set(std::vector<int>&& v) { _v = std::move(v); }

Right? Is it always best to define both in order to handle temporaries that can be move? So all setters will be doubled? Thanks.

cigien
  • 57,834
  • 11
  • 73
  • 112
lucmobz
  • 453
  • 3
  • 8

2 Answers2

5

If you know the type has an efficient move constructor (which std::vector<int> does), then you can just take by value and move from the argument:

void set(std::vector<int> v) { _v = std::move(v); }

That way, when called with an lvalue it will be copied, but when called with an rvalue it will be moved.

Artyer
  • 31,034
  • 3
  • 47
  • 75
  • 1
    This is exactly how clang-tidy told me to do. – 康桓瑋 Jan 29 '21 at 08:25
  • 2
    I heard that's what they do in Google. source: [Chrome Developers](https://youtu.be/UNJrgsQXvCA?t=235) – Kao Jan 29 '21 at 08:43
  • Is that using template forwarding reference is a bad idea here? – 康桓瑋 Jan 29 '21 at 09:18
  • @Artyer: It might still have debate between [pass-by-value-vs-pass-by-rvalue-reference](https://stackoverflow.com/questions/37935393/pass-by-value-vs-pass-by-rvalue-reference) – Jarod42 Jan 29 '21 at 09:53
  • 1
    @康桓瑋: I expose the different possibilities in [my answer](https://stackoverflow.com/a/61601352/2684539) of *"Taking sink parameters by rvalue reference instead of by value to enforce performant usage of interfaces"*. So it improves performance. Readability is questionable though. – Jarod42 Jan 29 '21 at 09:57
0

I would vote against defining a single setter for passing arguments by value:

  1. When a value needs to be copied, it will always have to allocate new memory. Taking an argument in by reference and assigning it to the internal variable could reuse existing capacity.
    void set(const std::vector<int>& v) 
    {
        // this->v may already be allocated, reuse it
        this->v = v;
    }
  1. Setters taking in an rvalue reference can often be declated noexcept. Although technically exceptions thrown while passing an argument don't violate the noexcept guarantee, it may be counter-intuitive for users.
    void set(std::vector<int>&& v) noexcept
    {
        static_assert(std::is_nothrow_move_assignable_v<decltype(v)>);
        this->v = std::move(v);
    }

    /*
    // Technically also works but calling a noexcept function which potentially 
    // may throw an exception while making a copy of an argument can be confusing
    void set(std::vector<int> v) noexcept
    {       
        this->v = std::move(v);
    } */

When a setter has only one parameter I would recommend two setters taking lvalue and rvalue references. Setters are typically simple methods, so there will be (almost) no code duplication. In case of a complex setter, you may internally define a template method (or a free function) taking in a universal reference:


void MyClass::set(const std::vector<int>& v)
{
    setHelper(v);
}

void MyClass::set(std::vector<int>&& v)
{
    setHelper(std::move(v));
}

template<typename T>
void MyClass::setHelper(T&& v)
{
    // Some complex code ...

    this->v = std::forward<T>(v);

    // More complex code ...
}
mentalmushroom
  • 2,261
  • 1
  • 26
  • 34
  • What's the downside of having just `set(std::vector) noexcept`? Just that the `noexcept` is easy to misunderstand? – Mooing Duck Jun 12 '23 at 16:20
  • @MooingDuck Basically, yes. When users call a method which is noexcept, they don't expect it to throw, regardless of the fact that an exception originates in passing an argument rather than the function itself. Herb Sutter advised not to declare such a function noexcept. – mentalmushroom Jun 12 '23 at 17:24