Return by value vs return by reference?

Question

I wish to seek a bit of feedback (to help me see if my position on the issue is correct or not) regarding returning by const reference vs. returning by value (for performance reasons).

To elaborate:

#include "BigDataStruct.h"

class DataGiver
{
  public:
    BigDataStruct getByValue() const { return myData; };
    const BigDataStruct& getByConstRef() const { return myData; };

  private:
    BigDataStruct myData;
}

My gut feeling says there may (or may not) be a slight performance increase for getByConstRef() over getByValue() in situations like:

const BigDataStruct& someData = someDataGiver.getByConstRef();

but there won't be (hardly) any for calls like:

BigDataStruct someData = someDataGiver.getByConstRef();

Also - which gives me some discomfort - is that in the first scenario someData is tied to actual member in someDataGiver, in the sense that if the member there changes someData also changes and if someDataGiver expires/dies someData becomes... undefined?

As such I try to discourage my coworkers from returning by const reference (unless we actually WANT a reference to the object for other reasons - e.g. in singleton factories). Do I have a point here or am I merely nitpicking and annoying my coworkers for no good reason?

(I am aware of the rule of thumb for optimization: 1) Don't do it 2) Don't do it yet - and use a profiler before actually doing it)

Answer 1

Since it's a feedback request, here are my 2 cents. There are two aspects I would like to focus on: performance and safety.

Performance

Returning by reference provides performance improvement by removing extra-copying. Amount of improvement strongly depends on the usage: how big is the object, how tight is the loop, etc.

To bring in some data: here's a benchmark on-line (note: BigDataStruct holds char[256]):

• Returning by reference has an impact. It's up to you to decide if it is significant on case-to-case basis.

• When doing BigDataStruct someData = someDataGiver.getByConstRef(); performance is the same

Safety

Returning by reference is not memory-safe and is not thread-safe since it provides aliasing to data (BigDataStruct) that could be potentially modified elsewhere (e.g., in DataGiver).

To illustrate it with an example:

const BigDataStruct& someData = someDataGiver.getByConstRef();
// do something with someData
someDataGiver.modifyData(); //<= someData is now changed!
// doing same things with someData can have different outcome

You might think: no sane developer will write anything like that. But here's one that can happen quite often (happened with me):

const BigDataStruct& someData = 
    DataGiver().getByConstRef(); // Bam! A wild dangling reference appears!

Conclusions

Returning by reference can improve performance. But it has few nasty pitfalls that developer has to keep track of. Whether to discourage it or not is a strong recommendation to make. IMO defaulting to returning-by-value and motivating returning-by-reference could be a balanced approach. Also explicitly stating that the value is returned by reference is a good idea (which your team already does with ByConstRef postfix).

Update: as @LightnessRacesinOrbit mentioned in the comment (also see answer by Pavlo K): if you don't have the control of all the getter usages (e.g., developing a library or having external users) returning by reference provides more flexibility by delegating the choice to the final user.

Answer 2

IMO this is natural way for c++ to return references and let user of сlass to decide how to deal with it.

For example stl containers usually return elements by reference, and that's up to developer copy it to local variable if needed, or use by reference (if developer fully controls element's lifecicle). (Consider std::vector as an example).

So, if there are no other reasons (like making DataGiver thread-safe, etc), I would return by const ref.