2

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.

2 Answers 2

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.

5
  • 1
    This is exactly how clang-tidy told me to do.
    – 康桓瑋
    Commented Jan 29, 2021 at 8:25
  • 2
    I heard that's what they do in Google. source: Chrome Developers
    – Kao
    Commented Jan 29, 2021 at 8:43
  • Is that using template forwarding reference is a bad idea here?
    – 康桓瑋
    Commented Jan 29, 2021 at 9:18
  • @Artyer: It might still have debate between pass-by-value-vs-pass-by-rvalue-reference
    – Jarod42
    Commented Jan 29, 2021 at 9:53
  • 1
    @康桓瑋: I expose the different possibilities in my answer 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
    Commented Jan 29, 2021 at 9: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 ...
}

2
  • What's the downside of having just set(std::vector<int>) noexcept? Just that the noexcept is easy to misunderstand? Commented Jun 12, 2023 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. Commented Jun 12, 2023 at 17:24

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.