How to pass parameters correctly?
Asked Answered
L

5

116

I am a C++ beginner but not a programming beginner. I'm trying to learn C++(c++11) and it's kinda unclear for me the most important thing: passing parameters.

I considered these simple examples:

  • A class that has all its members primitive types:
    CreditCard(std::string number, int expMonth, int expYear,int pin):number(number), expMonth(expMonth), expYear(expYear), pin(pin)

  • A class that has as members primitive types + 1 complex type:
    Account(std::string number, float amount, CreditCard creditCard) : number(number), amount(amount), creditCard(creditCard)

  • A class that has as members primitive types + 1 collection of some complex type: Client(std::string firstName, std::string lastName, std::vector<Account> accounts):firstName(firstName), lastName(lastName), accounts(accounts)

When I create an account, I do this:

    CreditCard cc("12345",2,2015,1001);
    Account acc("asdasd",345, cc);

Obviously the credit card will be copied twice in this scenario. If I rewrite that constructor as

Account(std::string number, float amount, CreditCard& creditCard) 
    : number(number)
    , amount(amount)
    , creditCard(creditCard)

there will be one copy. If I rewrite it as

Account(std::string number, float amount, CreditCard&& creditCard) 
    : number(number)
    , amount(amount)
    , creditCard(std::forward<CreditCard>(creditCard))

There will be 2 moves and no copy.

I think sometimes you may want to copy some parameter, sometimes you don't want to copy when you create that object.
I come from C# and, being used to references, it's a bit strange to me and I think there should be 2 overloads for each parameter but I know I am wrong.
Are there any best practices of how to send parameters in C++ because I really find it, let's say, not trivial. How would you handle my examples presented above?

Lampley answered 24/3, 2013 at 15:44 Comment(5)
Meta: I can't believe somebody just asked a good C++ question. +1.Infarct
FYI, std::string is a class, just like CreditCard, not a primitive type.Everything
Due to the string confusion, though unrelated, you should know that a string literal, "abc", is not of type std::string, not of type char */const char *, but of type const char[N] (with N=4 in this case due to three characters and a null). That's a good, common misconception to get out of the way.Everything
@chuex: Jon Skeet is all over C# questions, far more rarely C++.Screenplay
Highly related: How to pass object to functions in C++?Pastose
C
166

THE MOST IMPORTANT QUESTION FIRST:

Are there any best practices of how to send parameters in C++ because I really find it, let's say, not trivial

If your function needs to modify the original object being passed, so that after the call returns, modifications to that object will be visible to the caller, then you should pass by lvalue reference:

void foo(my_class& obj)
{
    // Modify obj here...
}

If your function does not need to modify the original object, and does not need to create a copy of it (in other words, it only needs to observe its state), then you should pass by lvalue reference to const:

void foo(my_class const& obj)
{
    // Observe obj here
}

This will allow you to call the function both with lvalues (lvalues are objects with a stable identity) and with rvalues (rvalues are, for instance temporaries, or objects you're about to move from as the result of calling std::move()).

One could also argue that for fundamental types or types for which copying is fast, such as int, bool, or char, there is no need to pass by reference if the function simply needs to observe the value, and passing by value should be favored. That is correct if reference semantics is not needed, but what if the function wanted to store a pointer to that very same input object somewhere, so that future reads through that pointer will see the value modifications that have been performed in some other part of the code? In this case, passing by reference is the correct solution.

If your function does not need to modify the original object, but needs to store a copy of that object (possibly to return the result of a transformation of the input without altering the input), then you could consider taking by value:

void foo(my_class obj) // One copy or one move here, but not working on
                       // the original object...
{
    // Working on obj...

    // Possibly move from obj if the result has to be stored somewhere...
}

Invoking the above function will always result in one copy when passing lvalues, and in one moves when passing rvalues. If your function needs to store this object somewhere, you could perform an additional move from it (for instance, in the case foo() is a member function that needs to store the value in a data member).

In case moves are expensive for objects of type my_class, then you may consider overloading foo() and provide one version for lvalues (accepting an lvalue reference to const) and one version for rvalues (accepting an rvalue reference):

// Overload for lvalues
void foo(my_class const& obj) // No copy, no move (just reference binding)
{
    my_class copyOfObj = obj; // Copy!
    // Working on copyOfObj...
}

// Overload for rvalues
void foo(my_class&& obj) // No copy, no move (just reference binding)
{
    my_class copyOfObj = std::move(obj); // Move! 
                                         // Notice, that invoking std::move() is 
                                         // necessary here, because obj is an
                                         // *lvalue*, even though its type is 
                                         // "rvalue reference to my_class".
    // Working on copyOfObj...
}

The above functions are so similar, in fact, that you could make one single function out of it: foo() could become a function template and you could use perfect forwarding for determining whether a move or a copy of the object being passed will be internally generated:

template<typename C>
void foo(C&& obj) // No copy, no move (just reference binding)
//       ^^^
//       Beware, this is not always an rvalue reference! This will "magically"
//       resolve into my_class& if an lvalue is passed, and my_class&& if an
//       rvalue is passed
{
    my_class copyOfObj = std::forward<C>(obj); // Copy if lvalue, move if rvalue
    // Working on copyOfObj...
}

You may want to learn more about this design by watching this talk by Scott Meyers (just mind the fact that the term "Universal References" that he is using is non-standard).

One thing to keep in mind is that std::forward will usually end up in a move for rvalues, so even though it looks relatively innocent, forwarding the same object multiple times may be a source of troubles - for instance, moving from the same object twice! So be careful not to put this in a loop, and not to forward the same argument multiple times in a function call:

template<typename C>
void foo(C&& obj)
{
    bar(std::forward<C>(obj), std::forward<C>(obj)); // Dangerous!
}

Also notice, that you normally do not resort to the template-based solution unless you have a good reason for it, as it makes your code harder to read. Normally, you should focus on clarity and simplicity.

The above are just simple guidelines, but most of the time they will point you towards good design decisions.


CONCERNING THE REST OF YOUR POST:

If i rewrite it as [...] there will be 2 moves and no copy.

This is not correct. To begin with, an rvalue reference cannot bind to an lvalue, so this will only compile when you are passing an rvalue of type CreditCard to your constructor. For instance:

// Here you are passing a temporary (OK! temporaries are rvalues)
Account acc("asdasd",345, CreditCard("12345",2,2015,1001));

CreditCard cc("12345",2,2015,1001);
// Here you are passing the result of std::move (OK! that's also an rvalue)
Account acc("asdasd",345, std::move(cc));

But it won't work if you try to do this:

CreditCard cc("12345",2,2015,1001);
Account acc("asdasd",345, cc); // ERROR! cc is an lvalue

Because cc is an lvalue and rvalue references cannot bind to lvalues. Moreover, when binding a reference to an object, no move is performed: it's just a reference binding. Thus, there will only be one move.


So based on the guidelines provided in the first part of this answer, if you are concerned with the number of moves being generated when you take a CreditCard by value, you could define two constructor overloads, one taking an lvalue reference to const (CreditCard const&) and one taking an rvalue reference (CreditCard&&).

Overload resolution will select the former when passing an lvalue (in this case, one copy will be performed) and the latter when passing an rvalue (in this case, one move will be performed).

Account(std::string number, float amount, CreditCard const& creditCard) 
: number(number), amount(amount), creditCard(creditCard) // copy here
{ }

Account(std::string number, float amount, CreditCard&& creditCard) 
: number(number), amount(amount), creditCard(std::move(creditCard)) // move here
{ }

Your usage of std::forward<> is normally seen when you want to achieve perfect forwarding. In that case, your constructor would actually be a constructor template, and would look more or less as follows

template<typename C>
Account(std::string number, float amount, C&& creditCard) 
: number(number), amount(amount), creditCard(std::forward<C>(creditCard)) { }

In a sense, this combines both the overloads I've shown previously into one single function: C will be deduced to be CreditCard& in case you are passing an lvalue, and due to the reference collapsing rules, it will cause this function to be instantiated:

Account(std::string number, float amount, CreditCard& creditCard) : 
number(num), amount(amount), creditCard(std::forward<CreditCard&>(creditCard)) 
{ }

This will cause a copy-construction of creditCard, as you would wish. On the other hand, when an rvalue is passed, C will be deduced to be CreditCard, and this function will be instantiated instead:

Account(std::string number, float amount, CreditCard&& creditCard) : 
number(num), amount(amount), creditCard(std::forward<CreditCard>(creditCard)) 
{ }

This will cause a move-construction of creditCard, which is what you want (because the value being passed is an rvalue, and that means we are authorized to move from it).

Costello answered 24/3, 2013 at 15:55 Comment(12)
Is it wrong to say that for non-primitive parameter types it is ok to always use the template version with forward?Lampley
@JackWillson: I would say you should resort to the template version only when you really have concerns on the performance of moves. See this question of mine, which has a good answer, for more information. In general, if you don't have to make a copy and only need to observe, take by ref to const. If you need to modify the original object, take by ref to non-`const. If you need to make a copy and moves are cheap, take by value and then move.Costello
I was actually reading your question because now that you, and the others that replayed, made thinks more clear to me about constructing objects, i was asking myself the same thing about getters/setters and i reached to your question.Lampley
I think there is no need to move in the third code sample. You could just use obj instead of making a local copy to move into, no?Hussey
nice answer, but there are one thing maybe need to be fixed?my_class copyOfObj = obj; // Move would not move the contents of the obj because the rvalue with name is lvalue. You should wrap it by std::move or std::forward(I would prefer std::move in this case) if you want to move itMctyre
@AndyProwl: You got my +1 hours ago, but re-reading your (excellent) answer, I'd like to point out two things: a) by-value does not always create a copy (if it is not moved). The object could be constructed in-place on the callers site, especially with RVO/NRVO this even works more often than one would first think. b) Please point out that in the last example, std::forward may be called only once. I've seen people putting it into loops, etc. and since this answer will be seen by a lot of beginners, there should IMHO be a fat "Warning!"-label to help them avoid this trap.Marylinmarylinda
@StereoMatching: Absolutely correct, I forgot to add the std::move over there. I will edit asap (typing from mobile now). Thank you for pointing out!Costello
@DanielFrey: Good observations, I will edit the answer and mention both points. Thank you for that!Costello
@Andy: I don't think it harms the accuracy of your answer, but why say "resort to the template version"? What disadvantage does it have compared with pass-by-value-and-move other than that, being a template, it must be defined in a header? I wonder if perhaps C++11 style will develop so that perfect-forwarded functions are the default, and we resort to pass-by-value-and-move when our functions aren't in headers (which for some people is always, but I think most C++ programmers do put simple constructors in class definitions). As soon as it's commonly used it will become easy to read.Screenplay
@SteveJessop: I agree that this idiom should become more popular, and I personally find it appealing. However, I got repeatedly downvoted/criticized for proposing it as an alternative to pass-by-value-then-move, pass-by-const-ref-then-copy etc., with the motivation that those approaches are "more idiomatic" (see, for instance, this). So I prefer to be more careful now.Costello
@SteveJessop: And apart from that, there are technical issues (not necessarily problems) with forwarding functions (especially constructors taking one argument), mostly the fact that they accept arguments of any type and may defeat the std::is_constructible<> type trait unless they are properly SFINAE-constrained - which may not be trivial for some.Costello
Very comprehensive and well-structured answer, kudos @AndyProwl!Endorsement
S
11

First, let me correct some details. When you say the following:

there will be 2 moves and no copy.

That is false. Binding to an rvalue reference is not a move. There is only one move.

Additionally, since CreditCard is not a template parameter, std::forward<CreditCard>(creditCard) is just a verbose way of saying std::move(creditCard).

Now...

If your types have "cheap" moves, you may want to just make your life easy and take everything by value and "std::move along".

Account(std::string number, float amount, CreditCard creditCard)
: number(std::move(number),
  amount(amount),
  creditCard(std::move(creditCard)) {}

This approach will yield you two moves when it could yield only one, but if the moves are cheap, they may be acceptable.

While we are on this "cheap moves" matter, I should remind you that std::string is often implemented with the so-called small string optimisation, so its moves may not be as cheap as copying some pointers. As usual with optimisation issues, whether it matters or not is something to ask your profiler, not me.

What to do if you don't want to incur those extra moves? Maybe they prove too expensive, or worse, maybe the types cannot actually be moved and you might incur extra copies.

If there is only one problematic parameter, you can provide two overloads, with T const& and T&&. That will bind references all the time until the actual member initialisation, where a copy or move happens.

However, if you have more than one parameter, this leads to an exponential explosion in the number of overloads.

This is a problem that can be solved with perfect forwarding. That means you write a template instead, and use std::forward to carry along the value category of the arguments to their final destination as members.

template <typename TString, typename TCreditCard>
Account(TString&& number, float amount, TCreditCard&& creditCard)
: number(std::forward<TString>(number),
  amount(amount),
  creditCard(std::forward<TCreditCard>(creditCard)) {}
Singletary answered 24/3, 2013 at 16:4 Comment(2)
There's a problem with the template version: the user can't write Account("",0,{brace, initialisation}) anymore.Lanthanum
@Lanthanum ah, true. That is truly annoying, and I don't think there is an easy scalable workaround.Singletary
N
6

First of all, std::string is quite a hefty class type just like std::vector. It's certainly not primitive.

If you're taking any large moveable types by value into a constructor, I would std::move them into the member:

CreditCard(std::string number, float amount, CreditCard creditCard)
  : number(std::move(number)), amount(amount), creditCard(std::move(creditCard))
{ }

This is exactly how I would recommend implementing the constructor. It causes the members number and creditCard to be move constructed, rather than copy constructed. When you use this constructor, there will be one copy (or move, if temporary) as the object is passed into the constructor and then one move when initialising the member.

Now let's consider this constructor:

Account(std::string number, float amount, CreditCard& creditCard)
  : number(number), amount(amount), creditCard(creditCard)

You're right, this will involve one copy of creditCard, because it is first passed to the constructor by reference. But now you can't pass const objects to the constructor (because the reference is non-const) and you can't pass temporary objects. For example, you couldn't do this:

Account account("something", 10.0f, CreditCard("12345",2,2015,1001));

Now let's consider:

Account(std::string number, float amount, CreditCard&& creditCard)
  : number(number), amount(amount), creditCard(std::forward<CreditCard>(creditCard))

Here you've shown a misunderstanding of rvalue references and std::forward. You should only really be using std::forward when the object you're forwarding is declared as T&& for some deduced type T. Here CreditCard is not deduced (I'm assuming), and so the std::forward is being used in error. Look up universal references.

Namaqualand answered 24/3, 2013 at 15:54 Comment(0)
S
1

It's kinda unclear for me the most important thing: passing parameters.

  • If you want to modify the variable passed inside the function / method
    • you pass it by reference
    • you pass it as a pointer (*)
  • If you want to read the value / variable passed inside the function / method
    • you pass it by const reference
  • If you want to modify the value passed inside the function / method
    • you pass it normally by copying the object (**)

(*) pointers may refers to dynamically allocated memory, therefore when possible you should prefer references over pointers even if references are, in the end, usually implemented as pointers.

(**) "normally" means by copy constructor (if you pass an object of the same type of the parameter) or by normal constructor (if you pass a compatible type for the class). When you pass an object as myMethod(std::string), for example, the copy constructor will be used if an std::string is passed to it, therefore you have to make sure that one exists.

Stream answered 24/3, 2013 at 15:57 Comment(0)
L
1

I use a quite simple rule for general case: Use copy for POD (int, bool, double,...) and const & for everything else...

And wanting to copy or not, is not answered by the method signature but more by what you do with the paramaters.

struct A {
  A(const std::string& aValue, const std::string& another) 
    : copiedValue(aValue), justARef(another) {}
  std::string copiedValue;
  const std::string& justARef; 
};

precision for pointer : I almost never used them. Only advantage over & is that they can be null, or re-assigned.

Leonleona answered 24/3, 2013 at 15:57 Comment(4)
"I use a quite simple rule for general case: Use copy for POD (int, bool, double,...) and const & for everything else." No. Just No.Stream
May be adding, if you want to modify a value using a & (no const). Else I don't see... simplicity is good enough. But if you said so...Leonleona
Some times you want to modify by reference primitive types, and sometimes you need to make a copy of objects and sometimes you have to move objects. You can't just reduce everything to POD > by value and UDT > by reference.Stream
Ok, this is just what I have added after. May be a too fast answer.Leonleona

© 2022 - 2024 — McMap. All rights reserved.