0

I came across a C++17 code base where functions always accept parameters by value, even if a const reference would work, and no semantic reason for passing by value is apparent. The code then explicitly uses a std::move when calling functions. For instance:

A retrieveData(DataReader reader) // reader could be passed by const reference.
{
    A a = { };
    a.someField = reader.retrieveField();   // retrieveField is a const function.
    return a;
}
    
auto someReader = constructDataReader();
auto data = retrieveData(std::move(someReader)); // Calls explicitly use move all the time.

Defining functions with value parameters by default and counting on move semantics like this seems like a bad practice, but is it? Is this really faster/better than simply passing lvalues by const reference, or perhaps creating a && overload for rvalues if needed?

I'm not sure how many copies modern compilers would do in the above example in case of a call without an explicit move on an lvalue, i.e. retrieveData(r).

I know a lot has been written on the subject of moving, but would really appreciate some clarification here.

w128
  • 4,680
  • 7
  • 42
  • 65
  • 4
    It's very difficult to say anything about the goodness of this without reasonably intimate knowledge about the context. – molbdnilo Feb 04 '22 at 15:12
  • 1
    Does this answer your question? [When is a const reference better than pass-by-value in C++11?](https://stackoverflow.com/questions/24543330/when-is-a-const-reference-better-than-pass-by-value-in-c11) – jjramsey Feb 04 '22 at 15:14
  • ¿Have you asked people who wrote it? I can easily image a situation when it would be better to take an objects by value (because it is not supposed to be used after call for example) and constructing them outside of the function arguments list and then moving is required to have deterministic order of function argument evaluation. With just a single argument it doesn't make much sense though. – user7860670 Feb 04 '22 at 15:16
  • Calling a move constructor is not the same as passing as const reference. Maybe there is a reason for this, it means that you are not supposed to continue using this object in the caller. – mmomtchev Feb 04 '22 at 15:19
  • If `retrieveData` is really supposed to "consume" the reader, making the argument meaningless for the caller after it returns, then this way makes a lot of sense. Then again, it's impossible to tell from this snippet whether it is. – molbdnilo Feb 04 '22 at 15:38

0 Answers0