best practice for getter

[sbowman-mitre]

In the safety module, there is an example recommending using const to indicate an immutable getter method:
std::string get_value() const { /* etc */ }

However, I think that best practices actually recommend going slightly further by also returning a const reference. Like this:
std::string const &get_value() const { /* etc */ }

Can anyone confirm or deny this practice? Is it a best practice?

[Jofagi]

While it doesn't have the most upvotes, I find this argument against const references pretty convincing:
It potentially gives better performance but the caller might store the result as a reference and this can lead to problems, when the original object goes out of scope. In my opinion, returning by value is fine most of the time.

[sbowman-mitre]

Ah, yes, that is a good discussion to consider. Thanks for linking it. Now I'm unsure as well.

[lefticus]

IMO, the argument would go this way:

1. const methods should be considered thread safe (according to Sutter and Meyers, see various talks from Going Native, CppCon and Effective Modern C++)
2. If you return by const & it would be impossible to return the value in a thread safe way

That said, I personally use const & on data that I know is immutable, and by value when dealing with thread safety issues on mutable data. The "dangling reference" issue isn't a big one to me - if you aren't thinking about the lifetime of your objects there are plenty of other ways to get yourself in trouble in C++...

I'm reluctant to make a specific recommendation one way or the other in this doc (I wasn't thinking about anything particular when I wrote that example, I think it just happened). I'll probably just link to this issue inside of the doc for people to reference the discussion, like I did on some of the style stuff.

[vladon]

Don't help the compiler

For example, you have a method (getter):

// ...
private:
  std::string m_internal_string;

public:
  const std::string& get_internal_string()
  {
    return m_internal_string;
  }

You want to avoid copy's, that's why you recommend return by const ref.

How will you use this?

For example:

std::string my_string = my_object.get_internal_string();

But there any way will be a copy.

Ok, you can say, that you will store it in const std::string&:

const std::string& my_string_ref = my_object.get_internal_string();

But what you will do with this const ref? Pass it to another method which will accept const std::string& and then that method will copy it to internal variable?

Ok, you can say that there will be one copy instead of multiple copies on each passing. No! There is such thing as copy elision. Any modern (not older than 5 years) compiler supports it (with optimization on).

And if you return by const ref, you cannot use non-const methods while rvalue-ing, for example, you cannot do this with const ref:

std::string my_string = my_object.get_internal_string().append("my modification");

So, what advantage can you get with return by const ref? Only avoiding copies. But copy elision eliminates unnecessary copies.
What negatives? No thread-safe. No guarantee that data will not be changed after return. No guarantee that ref will not become dangling.

Always return objects by value, unless you have strong reason not to.

[Manu343726]

But what if the guy taking the string only wants to look at the string? Then by value introduces overhead since the string will be copied, as in:

void print(const std::string& str)
{
    std::cout << str << "\n";
}

print(my_object.get_internal_string());

I wrote a post a month ago about how to design with this problems in mind (API vs internals): http://manu343726.github.io/raii-for-api-naked-ptrs-for-internals/

[patricer]

I've done both to show the impact on a matrix class made of vectors of vectors. The difference in speed when returning a rwo by value / by const& is very significant.

The main problem with const& in my opinion comes when (a) the calling code uses a const reference for the method's result, and (b) two threads are involved. Then, it's possible that a thread thinks it «owns» (shares, of course) a «constant» while another thread modified the referenced object. Nasty bug that return-by-value avoids.

One could claim that the bug, concretely, stems from the fact that the client code has kept a const& to the return value instead of making a copy (which would be the «normal» thing to do, and would occur if auto had been used). In terms of best practice, I can't push for return-by-value in most cases due to the performance loss, but there's probably something to be said for proper behavior on the part of the calling code.

[lefticus]

Reclosing this with commit d8ea5ad It's not overly verbose but I think captures the issue.