Should all/most setter functions in C++11 be written as function templates accepting universal references?

Question

Consider a class X with N member variables, each of some copiable and movable type, and N corresponding setter functions. In C++98, the definition of X would likely look something like this:

class X
{
public:
    void set_a(A const& a) { _a = a; }
    void set_b(B const& b) { _b = b; }
    ...
private:
    A _a;
    B _b;
    ...
};

Setter functions of class X above can bind both to lvalue and to rvalue arguments. Depending on the actual argument, this might result in the creation of a temporary and will eventually result in a copy assignment; due to this, non-copiable types are not supported by this design.

With C++11 we have move semantics, perfect forwarding, and universal references (Scott Meyers's terminology), which allow for a more efficient and generalized use of setter functions by rewriting them this way:

class X
{
public:
    template<typename T>
    void set_a(T&& a) { _a = std::forward<T>(a); }

    template<typename T>
    void set_b(T&& b) { _b = std::forward<T>(b); }
    ...
private:
    A _a;
    B _b;
    ...
};

Universal references can bind to const/non-const, volatile/non-volatile, and to any convertible type in general, avoiding the creation of temporaries and passing values straight to operator =. Non-copiable, movable types are now supported. Possibly undesired bindings can be eliminated either through static_assert or through std::enable_if.

So my question is: as a design guideline, should all (let's say, most) setter functions in C++11 be written as function templates accepting universal references?

Apart from the more cumbersome syntax and the impossibility of using Intellisense-like helper tools when writing code in those setter functions, are there any relevant disadvantages with the hypothetical principle "write setter functions as function templates accepting universal references whenever possible"?

To me, yes they should if you cannot pass by value. Also many constructors should be variadic and take universal references (`struct foo : bar { template foo (Ts&&... ts) : bar (std::forward (ts)...) {} };`). Getters and setters are rare actually, so intellisense is not a big deal here. — Alexandre C., Jan 07 '13 at 14:09
@AlexandreC. that example seems like a bad one. Did you know that constructor will get picked over a regular copy constructor (i.e. declared as `foo(foo const&)`) for making copies (http://flamingdangerzone.com/cxx11/2012/06/05/is_related.html)? It doesn't forward initializer lists properly either. On the other hand, the language now has inherited ctors (compiler writers, could you pretty please with sugar on top implement these?), so you can get something much more robust than that ctor that with a simple `using bar::bar;`. — R. Martinho Fernandes, Jan 07 '13 at 14:12
Why (on earth) is this thing not a public field? Especially if you wanted to be able to do anything with it that you could you do with a normal field. If you want a write-only property, make it a _method_ (a.o.t. a _setter_) — sehe, Jan 07 '13 at 14:13
@sehe: those setter functions are intentionally simplistic, there might be something more going on in there than just assigning. — Andy Prowl, Jan 07 '13 at 14:15
@sehe: maybe a future implementation will not actually have an `a` field, but will still logically implement `set_a` by otherwise storing whatever attributes it needs from the specified instance of `A`. Or perhaps in future the value of the field will not be orthogonal to all other data members, so `set_a` might update something else too. I know, YAGNI, but if the class was called `URL` then I wouldn't necessarily want to commit to a public `protocol` data member of type `string` even though I am willing to commit to always having a `set_protocol` member function. — Steve Jessop, Jan 07 '13 at 14:33
@SteveJessop that's why I said: make it a _method_, not a _setter_. Implement the behaviour, don't expose internals. — sehe, Jan 07 '13 at 14:38
@sehe: I fail to see your point. I may have a member variable which requires *non-trivial* setting/getting (my example was just simplification, assignment could be just a part of what is going on there). Why shouldn't I have getter/setter functions for that? Of course I did not show getter functions in my example because they are irrelevant to the question I am asking, but this doesn't mean those properties are write-only. — Andy Prowl, Jan 07 '13 at 14:41
@sehe: My first answer comment was to your first question, "why not a field". To your second point, I don't see what the distinction is between your proposal and Andy's, other than that he chooses to use the term "setter" for such member functions, whereas you choose to call them "methods". IMO whether or not a member function is a "setter" is an implementation detail, it's not part of the interface and it doesn't really matter too much whether you call it a setter or not. — Steve Jessop, Jan 07 '13 at 14:45
@sehe Why (on earth) are too simple examples on Stack Overflow as bad as too complicated ones? I think you totally got the idea here. When we want to hide a member behind getter/setter, then *how* should we implement it? OP did provide a small example. Period. — leemes, Jan 07 '13 at 14:46
@leemes The thing is, with a a generic setter like that, you're not actually _hiding_ anything. The implementation will not be able to be changed without potentially breaking any client code. The interface is essentially like a wildcard. — sehe, Jan 07 '13 at 14:55
@sehe: why not? i am hiding the logics with which the variable gets set. — Andy Prowl, Jan 07 '13 at 14:56
@sehe What about firing an `a_changed()` event in the future for example? Or debugging the change of the property... — leemes, Jan 07 '13 at 15:14
[This might be helpful](http://stackoverflow.com/q/7592630/726300). Consider also that 'copiable' can be seen as a specialization of 'movable', so saying that a type is 'both movable and copiable' would be redundant. — Luc Danton, Jan 07 '13 at 16:04
@LucDanton: I don't think *copiable* is a specialization of *movable*. There are types that are *movable* but not *copiable* (e.g. `unique_ptr`) and types which are *copiable* but not *movable* (ok, can't think of a concrete example here, but you can apply `= delete` to a move constructor and make the object unmovable and yet copiable). — Andy Prowl, Jan 07 '13 at 16:15
@AndyProwl: Copyable *is* a special-case of Movable. Logically, copying an object is a valid implementation of a move (both construction and assignment). Language-wise, a `const &` reference can bind to an rvalue. So every Copyable type is also Movable. It may not be *efficiently* movable, though. — Steve Jessop, Jan 07 '13 at 16:43
@SteveJessop: it can, but if you declare the move constructor as `= delete`, having a copy constructor which accepts `const&` will not let you assign an rvalue, because overload resolution would select the deleted move constructor. That effectively makes the object copiable but not movable. — Andy Prowl, Jan 07 '13 at 16:47
@AndyProwl: Ah, good point. I missed that `=delete` prevents the less-good match from being selected, oops. — Steve Jessop, Jan 07 '13 at 16:48
@SteveJessop: as a post scriptum, I must admit that I find it hard to figure out a concrete use case for a type which is copyable but not movable. Nevertheless, it's legal to have one... — Andy Prowl, Jan 07 '13 at 16:52
Andy has taken up the issue in this question: http://stackoverflow.com/questions/14323093/are-there-any-use-cases-for-a-class-which-is-copyable-but-not-movable. I note that my original claim (that copyable is a special-case of movable) is correct if we take "Copyable" to mean `CopyConstructible` and/or `CopyAssignable` and "Movable" to mean `MoveConstructible` and/or `MoveAssignable`. But the reason I gave in my comment above was motivation at best and completely spurious at worst -- it has nothing to do with logical use or reference binding, it's an explicit requirement of the concepts. — Steve Jessop, Jan 14 '13 at 19:28

score 36 · Accepted Answer · answered Jan 07 '13 at 14:10

36

You know the classes A and B, so you know if they are movable or not and if this design is ultimately necessary. For something like std::string, it's a waste of time changing the existing code unless you know you have a performance problem here. If you're dealing with auto_ptr, then it's time to rip it out and use unique_ptr.

It's usually preferred now to take arguments by value if you don't know anything more specific- such as

void set_a(A a) { _a = std::move(a); }

This permits the use of any of the constructors of A without requiring anything except movability and offers a relatively intuitive interface.

answered Jan 07 '13 at 14:10

Puppy

138,897
33
232
446

I see your point, but this would cause an unnecessary move if `set_a` is invoked with an rvalue, isn't it so? – Andy Prowl Jan 07 '13 at 14:17
@AndyProwl: I think not necessarily, since I think the move from the rvalue to `a` can be elided if the rvalue is a temporary. The move from there to `_a` can't be elided no matter what you do since it's assignment not construction. I suspect you're right in the case where someone calls `set_a(std::move(something))` though: barring inlining and "as-if" optimizations there will be two moves, whereas your template function moves only once. – Steve Jessop Jan 07 '13 at 14:38
@SteveJessop: you are right about elision, i did implicitly referr to rvalues obtained as the result of invoking `std::move`. should have mentioned that explicitly – Andy Prowl Jan 07 '13 at 14:44
6

OK, I will accept this answer and take the design principle I was looking for as follows: "*For setter functions like the ones shown in the example, if the type is movable and the performance of move construction is not an issue, then take arguments by value; otherwise, use universal references and rule out unwanted bindings through `std::enable_if` or `static_assert`; in any case, avoid taking arguments by const ref (again: for setter functions like the ones in the example)*". Thank you – Andy Prowl Jan 07 '13 at 16:05
17

This is false, for something like a std::string, accepting the argument by value causes a guaranteed allocation anytime the function is called with an lvalue. Whereas passing by const reference, the real work happens in the assignment inside the function body, since the real work happens in copy assignment and not copy construction, pre-allocated space can be reused if sufficiently large. You can benchmark this in a loop and you'll see that const ref crushes value for lvalue calls. If you want the speed boost for rvalues, you should implement an rvalue overload. – Nir Friedman Sep 30 '14 at 20:32
1

I doubt if "passing-by-value" + `std::move()` is a proper design for the problem. If so, why do many classes in the standard library provide both the `T const&` and the `T&&` overloads? Why do we need both overloads at the first place? – Siu Ching Pong -Asuka Kenji- Mar 30 '15 at 15:19
Where is the advantage of `by value` over an `rvalue` reference? – ted May 27 '15 at 19:04
Passing by value is a reasonable approach _only if_ the type is much cheaper to move than to copy, the body will *always* copy the parameter, *and* the method is often called with both lvalues and rvalues. If any of those conditions do not hold, then pass-by-value is a pessimisation rather than an optimisation. _However_ it still lets you write less code (or lets you avoid the template), so sometimes you might want to take that performance hit anyway for a maintenance cost reduction. Depends on your app and the method usage. – Miral Feb 28 '17 at 01:13
@NirFriedman , So, is `std::string`'s copy assignment cheaper than `std::string`'s copy construction? would you please provide some elaboration on why this is the case? – Mike Apr 15 '17 at 10:15
3

@Mike The copy constructor always triggers a memory allocation. The copy assignment only triggers memory allocation if the new contents are significantly larger than the old contents. Even assuming "tight" memory allocation, for a list of N distinctly sized strings, setting to each via copy construction is N allocations. Via copy assignment is O(logN). – Nir Friedman Apr 17 '17 at 16:50
@NirFriedman This is a bit of a specific to `std::string` and not a general pattern. For instance, your comment would not be applicable to a move-only class like `unique_ptr`. For small strings SBO is used and for large strings, copying the string probably dominates the memory allocation time anyway. Simplicity and uniformity is worth a minor runtime cost until the profiler tells you otherwise and when it does, you will implement something custom anyway. – Puppy Apr 16 '19 at 06:47
@Puppy Copy assignment being cheaper than copy construction is hardly specific to string. It applies to `vector` as well, probably to `filesystem::path`, it may apply to deque (not sure), etc. I would say that "it's usually preferred now", is highly subjective. Herb at cppcon a few years ago explicitly recommends const& + && for single argument setters, and by value only for constructors, for example. – Nir Friedman Apr 16 '19 at 18:18

Should all/most setter functions in C++11 be written as function templates accepting universal references?

1 Answers1

Linked