C++ Returning Reference to New Object

Question

I know there are similar questions to this but none of them give a definitive answer to my question...

Are both these okay in terms of best practices? Or should I be returning a pointer? And if not how should they be changed to follow best practices.

I want to return a reference to a new object from a function. My implementation is as follow:

MyClass& doSomething() {
   return *(new MyClass());
}

MyClass a = doSomething();

Is this okay because a new instance of MyClass is being allocated on the heap with new?

Or should I be making it constant (I'm not really sure when to do this or not)?

const MyClass& doSomething() {
    return *(new MyClass());
}

And if both these are wrong should I just be returning a pointer to the new object?

Thanks.

Answer 1

While this isn't exactly invalid, it's not a good idea.

MyClass& doSomething() {
   return *(new MyClass());
}

After you return, nobody has the original pointer, so nobody will ever delete it.* So it's a memory leak.

You should pretty much never write a new unless you have a corresponding delete—or, better, a smart pointer constructor.

---

Meanwhile, this line in your original code:

MyClass a = doSomething();

… is going to make a copy of the value anyway. Assuming that wasn't another bug that has to be fixed, why bother heap-allocating an object and returning a reference to copy and leak? Just return the object by value:

MyClass doSomething() {
   return MyClass();
}

Now you don't have to worry about deleting anything, because you never created anything on the heap.

---

Best practices can usually be summed up in the four letters RAII: Resource Acquisition Is Initialization. (And the corollary, that destruction is release.) If you have something that's impossible, or expensive, to pass around by value, then pass around some handle to it by value. For example:

unique_ptr<MyClass> doSomething() {
    return unique_ptr<MyClass>(new myClass());
}

unique_ptr<MyClass> a = doSomething();

Now it's just a pointer being copied around. The object itself gets created inside doSomething, and deleted whenever a goes out of scope (or, if you pass it along to another variable, whenever that goes out of scope, etc.).

On the other hand, if MyClass is just a handful of easily-copyable values**, just copy it.

---

* It's not impossible to ever delete it; you can always take a pointer to the reference and delete that. It's just very unlikely you will ever do so, and it will look awkward. If you want to pass pointers around, pass pointers around. If you don't want to pass pointers around, wrap ownership up in a class and pass the class around by value.

** By "easily-copyable" I mean that it's easy to copy them safely, and that you actually do so. For example, a raw pointer or a file handle is just a few bytes, and the default copy constructor will gladly copy them over for you… but then you end up with multiple references to the same heap object or file, and it's impossible to track who's in charge of deleting or closing it.