Another C++ learning moment: returning strings from functions

Question

I've got some basic questions about C++. Consider the following code in which I attempt to return a string.

const std::string&
NumberHolder::getValueString() {
    char valueCharArray[100];
    sprintf_s(valueCharArray,"%f",_value);
    std::string valueString(valueCharArray);
    return valueString;
}

I'm attempting to return a string with the value of a class member called _value. However I'm getting the warning that I'm trying to pass back a pointer to a local variable. This is of course a bad thing. If I understand C++ enough at this point, this means that the pointer I pass back will already have delete called on it by the time someone tries to use it. So I modify:

const std::string&
NumberHolder::getValueString() {
    char valueCharArray[100];
    sprintf_s(valueCharArray,"%f",_value);
    std::string valueString = new std::string(valueCharArray);
    return (*valueString);
}

This should create a pointer on the stack which will survive outside of this function. Two problems here though: 1) it doesn't compile anyway and I don't understand why (error = cannot convert from 'std::string *' to 'std::basic_string<_Elem,_Traits,_Ax>') and 2) This seems like a potential memory leak because I'm depending upon someone else to call delete on this guy. What pattern should I be using here?

Answer 1

You are getting this warning because you are returning a reference to the local string, not a copy of the local string. Once the function returns, the local string is destroyed and the reference that you returned is invalid. Thus, you need to return the string by value, not by reference:

std::string NumberHolder::getValueString()

Answer 2

You're defeating the point of having a std::string by allocating it on the heap!

Just return it by value like this:

std::string NumberHolder::getValueString()
{ 
    char valueCharArray[100]; 
    sprintf_s(valueCharArray,"%f",_value); 
    return std::string(valueCharArray); 
} 

Just about every compiler nowadays will do return value optimization (RVO) on the return statement, so no copies should be made. Consider the following:

NumberHolder holder;
// ...
std::string returnedString = holder.getValueString();

With RVO, the compiler will generate the code for the above implementation of NumberHolder::getValueString() such that std::string is constructed at the location of returnedString, so no copies are needed.

Answer 3

std::string *valueString = new std::string(valueCharArray);

You'd need to create a pointer variable to hold the result from new, since it returns the pointer. However, the ideal solution would really be just to return by value:

std::string NumberHolder::getValueString() {
    ...
    return std::string(valueCharArray);
}

Answer 4

Your 1st attempt is correct if you return a temp variable but bind it in a const reference.

const std::string NumberHolder::getValueString(){}

const std::string& val = NumberHolder::getValueString();

const . But your second attempt is dangerous, depending on somebody else to delete.