Should I return reference to heap object or return value?

Question

I have these two simple functions. I thought that func1 was a good solution since you pass an object by reference. My textbook gave func2 as the answer for the best solution. Is this only because you aren't deallocateing heapstr? What if I declared heapstr in main and then passed it to the function so I was able to delete it afterwards?

#include <iostream>

using namespace std;


string& func1(const string &str) {
    string* heapstr=new string();
    for (int i = 0; i < str.size(); ++i) {
        *heapstr += str[i];
    }
    return *heapstr;
}

string func2(const string &str) {
    string heapstr;
    for (int i = 0; i < str.size(); ++i) {
        heapstr += str[i];
    }
    return heapstr;
}

int main() {
    cout << func1("aaa") << endl;
    cout << func2("aaa") << endl;
}

Answer 1

Should I return reference to heap object or return value?

Return by value.

There are a lot of reasons why, but none are really related to performance, because the compiler is good enough at optimising things, and even if it wasn't, most programs are I/O-bound, i.e. the time you wait for data from files or network sockets eats up all your performance, not the time spent by CPU operations themselves.

See for example the "C++ Core Guidelines" by Herb Sutter and Bjarne Stroustrup, which say at section "Return containers by value (relying on move or copy elision for efficiency)":

Reason

To simplify code and eliminate a need for explicit memory management.

As for your two functions...

My textbook gave func2 as the answer for the best solution. Is this only because you aren't deallocateing heapstr?

The memory leak is one of the problems. But the point is simply that returning by value is simpler and less error-prone. It's all about correctness, not speed. You wouldn't return an int* if you could just return an int instead, would you?

What if I declared heapstr in main and then passed it to the function so I was able to delete it afterwards?

You would introduce a lot of possibilities for memory leaks, crashes and undefined behaviour into your code. It would become longer, harder to write, harder to read, harder to maintain, harder to debug and harder to justify in a code review. In return, you would gain absolutely nothing.

Answer 2

The text book is correct. (Shocker.)

Func1 is faulty in every respect in which it differs from func2. It allocates an object from the heap without regard to how that object will be deleted. Then it returns a reference to the new object, hiding the pointer that might have otherwise been used to delete it. There is no efficiency gain, in fact Func1 is probably a bit slower. In any case, recite after me: "Avoid Early Optimization."

Since the advent of the Standard Template Library, many moons ago, it is almost never best to use operator new. The last time I used operator new was ca. 2003, and I wrapped the pointer in the equivalent of what we now know as a unique_ptr. Before you use operator new, read all about learn all about smart pointers and RAII.