69

My question is simple: if I have some class Man and I want to define member function that returns man's name, which of the following two variants shall I prefer?

First:

string name();

Second:

void name(/* OUT */ string &name);

The first variant is kind of inefficient because it makes unnecessary copies (local variable -> return value -> variable in the left part of assignment).

The second variant looks pretty efficient but it makes me cry to write

string name;
john.name(name);

instead of simple

string name(john.name());

So, what variant shall I prefer and what is the proper trade-off between efficiency and convenience/readability?

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
tonytony
  • 1,994
  • 3
  • 20
  • 27
  • 7
    Side note, as `name()` looks like a query make it `const`: `string name() const;`. – hmjd May 11 '12 at 14:16
  • @hmjd Why the first `const`? The returned object ends up belonging to the client, and he should be free to do with it what he wants. – James Kanze May 11 '12 at 14:19
  • 3
    @JamesKanze, that is not part of the signature but rather my comment, it is follwed by a ':'. – hmjd May 11 '12 at 14:21
  • If it is just a simple getter accessing a corresponding member variable, a `const std::string&` might be an even better idea. – Christian Rau May 11 '12 at 14:30
  • @hmjd Sorry, then. In the font I'm using, the : doesn't show up very well at all. – James Kanze May 11 '12 at 14:31
  • @ChristianRau Not usually. It introduces issues of lifetime of the reference, etc. In a base class, it can create real problems for the derived class. Unless you actually want to support client code referring to internal data (and are prepared to document the validity of such references), it's best to avoid returning a reference. (If the documentation of the function doesn't specify the lifetime of the reference, then I would consider the code broken.) – James Kanze May 11 '12 at 14:34
  • One of the many questions on RVO: http://stackoverflow.com/questions/7596183/is-rvo-return-value-optimization-guaranteed-for-all-objects-in-gcc-compilers – Matthieu M. May 11 '12 at 14:49
  • @hmjd Nice remark, I usually do so. Thanks :) – tonytony May 11 '12 at 14:55
  • @ChristianRau Let it be `fullname()` function that sticks first name and last name together and returns the result. – tonytony May 11 '12 at 14:56

7 Answers7

54

It's a good question and the fact that you're asking it shows that you're paying attention to your code. However, the good news is that in this particular case, there's an easy way out.

The first, clean method is the correct way of doing it. The compiler will eliminate unnecessary copies, in most cases (usually where it makes sense).

EDIT (6/25/2016)

Unfortunately it seems that David Abaraham's site has been offline for a few years now and that article has been lost to the ethers (no archive.org copy available). I have taken the liberty of uploading my local copy as a PDF for archival purposes, and it can be found here.

Mahmoud Al-Qudsi
  • 28,357
  • 12
  • 85
  • 125
  • Yeah, it's a very well-written post that really hits the nail on the head. – Mahmoud Al-Qudsi May 11 '12 at 14:19
  • Thanks! I've read the article and was **extremely** confused but it really cleans things up. Now I have to clean up my _efficient_ code =) – tonytony May 11 '12 at 14:44
  • Of course, I would always use the 'second variant' for very heavyweight variables. I feel like this method could even be self-commenting because a reader would assume the datatype is heavyweight. least, my opinion. – user606723 May 11 '12 at 18:44
  • The article is about RVO, but it is worth mentioning that In C++11, no RVO is necessary, because move semantics will kick in. – Luca Venturini May 14 '23 at 16:43
27

use the first variant:

string name();

The compiler will most likely optimize out any unnecessary copies. See return value optimization.

In C++11, move semantics means that you don't perform a full copy even if the compiler doesn't perform RVO. See move semantics.

Also bear in mind that if the alternative is

void name(std::string& s);

then you have to specify very clearly what can happen to s and what values it can have when passed to the function, and probably either do a lot of validity checking, or simply overwrite the input entirely.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
9

Since you want to create a getter for a field of your class, maybe you should go like that: inline const std::string& name() const { return this->name; }

Since the name is returned as a const reference, it won't be modified outside the class, also no copy will be created by returning the name.

After that, if you want to manipulate the name you will have to do a copy.

Mesop
  • 5,187
  • 4
  • 25
  • 42
  • Thanks for the answer! It makes sense, but I considered some situation when name is computed inside the function, e.g. 'fullname()' function that returns the last name and first name stuck together. – tonytony May 11 '12 at 14:50
  • 2
    Yes, in that case you will have to return the value, as a reference to a local variable won't be valid after returning from the function. An alternative to you problem would be to compute the fullname everytime the first name or last name are modified (you can do that in the `setName` or `setFirstName` methods) and store it in a fullname field. That way, you won't have to recompute the fullname everytime you call `fullname()`, you just have to return const reference to the fullname field (as written in my answer). – Mesop May 11 '12 at 15:14
6

I would go with the first. Return value optimization and C++11 will remove any copying overhead.

flumpb
  • 1,707
  • 3
  • 16
  • 33
2

As we have move-semantics (in C++11), you can use this:

string name();

Even in C++03, this is almost good, as the compiler might optimize this (Search for Return Value Optimization).

Nawaz
  • 353,942
  • 115
  • 666
  • 851
2

Rule #1 of optimization: Measure, optimize, measure. Or, as Knuth said, "premature optimization is the root of all evil".

Unless you have a strong indication that simply returning std::string will significantly impact the performance of your software, just do so. If you can measure a significant impact, find the critical path, and optimize that. Don't make any funny, project-wide "optimizations" that likely result in little to no performance benefit, but negatively impact the readability, maintainability and robustness of your code.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
1

I think you should use first variant. Because this is simple getter method and such getter/setter approach is used everywhere.

Robert
  • 3,471
  • 3
  • 21
  • 24