Optimal way to write two methods that return a value and reference of the same item in C++

Question

Suppose that I have a class that has two methods. One of them should find an object in a container and return it by value, and the other one should return it by reference. Naturally, I want the first method to be const (in the sense of this being const) and the second one not to be.

Is there a way to re-use the code between these two methods without introducing a third method that they both rely on? Below I've given a toy example of the problem, where one should imagine that the find step is actually considerably more complicated.

In "possible implementation 1" below, there is an error due to the fact that I am calling (non-const) get_ref from within (const) get_value. In "possible implementation 2," the reference is created from a temporary copy of the value, which of course is a major problem.

In the case that a const reference is desired, then of course there is no problem, but assume that I actually want an ordinary reference.

// Header:

#include <string>
#include <map>
using std::string;
using std::map;

class Test {
    public:
        map< string, string > stuff;
        string & get_ref( const string key );
        string get_value( const string key ) const;
};


// Possible implementation 1:

string Test::get_value( const string key ) const {
  return get_ref( key );
}

string & Test::get_ref( const string key ) {
  return stuff.find( key )->second;
}


// Possible implementation 2 (obviously wrong, but here for the sake of pointing that out):

string Test::get_value( const string key ) const {
  return stuff.find( key )->second;
}

string & Test::get_ref( const string key ) {
  return get_value( key );
}

Think a little about what `get_ref` is returning in the second implementation. If you try to actually *build* it what does the compiler tell you? — Some programmer dude, Apr 18 '17 at 02:28
On an unrelated note, you should also think about what happens if the key is not found and the `find` function returns the `end` iterator. — Some programmer dude, Apr 18 '17 at 02:28
@Someprogrammerdude if you are alluding to the return of a temporary, the OP has already mentioned that as a reason not to use the second implementation — The Dark, Apr 18 '17 at 02:38
@Someprogrammerdude I clearly stated that the second implementation is obviously bad. Read my question. I also said that this is a toy example meant to illustrate the problem. An actual application would have error handling. — sasquires, Apr 18 '17 at 02:41
@TheDark Then I don't really see the meaning of including such clearly erroneous code, if the OP knows it will not work. To the OP: If you know that the second alternative won't work, then don't include it, it's just noise. Instead concentrate on the first alternative, and how to fix it. — Some programmer dude, Apr 18 '17 at 02:42
@Someprogrammerdude I guess you have a point, but I was mostly including it to point out that it isn't an option. I will add to the comment above it — sasquires, Apr 18 '17 at 02:44
As for a possible solution to the first implementation, you *know* that `get_ref` doesn't in itself modify the object. It is, for all practical reasons, constant. So you could easily [cast away `const`](http://en.cppreference.com/w/cpp/language/const_cast) of `this` in the `get_value` function to be able to call `get_ref`. ***However*** this doesn't solve the problem that `get_ref` itself is flawed since you have no safe case if `key` is not found. — Some programmer dude, Apr 18 '17 at 02:48
Possible duplicate of [Elegant solution to duplicate, const and non-const, getters?](http://stackoverflow.com/questions/856542/elegant-solution-to-duplicate-const-and-non-const-getters) — sasquires, Apr 26 '17 at 00:13

score 0 · Accepted Answer · answered Apr 18 '17 at 02:59

0

To directly answer the question, the only way to avoid using a third/private method is to cast the const-ness of this away.

Using the "possible implementation 1", alter as follows:

string Test::get_value( const string key ) const {
  return const_cast<Test*>(this)->get_ref( key );
}

Now, to look a bit broader at two other issues: firstly, you should alter get_ref() so it does something sensible if stuff.find( key ) returns end() (the one-past-the-end iterator). Options include throwing an exception or returning an empty string.

Secondly, a simple efficiency improvement is to note that the method parameters pass strings by value, which can be easily and safely changed to pass const string& key.

answered Apr 18 '17 at 02:59

Kiwi Nick

150
1
10

I think I was typing the above at the same time as @someprogrammerdude :-S – Kiwi Nick Apr 18 '17 at 03:01
Thanks, I guess I will go with a const_cast. I've commented above that I understand that there should be error handling. I just wanted to keep the example as brief as possible (one line in each function). As for the relative speed of passing by value or reference, my understanding is that for small, built-in types, this depends more on the compiler than anything -- but this is something I know very little about. – sasquires Apr 18 '17 at 15:48
As an aside to my own answer, I should also point out, when returning an empty string or similar - be careful. `return "";` is unsafe (returning reference to local/temp object). Better to declare a `static` member or static local variable and return that. – Kiwi Nick Apr 18 '17 at 23:29

Optimal way to write two methods that return a value and reference of the same item in C++

1 Answers1