How to return references to object created inside a method

Question

I am reasoning about the best approach to return references to objects created inside a method, like in the following situation:

class A{
public:
    A(){}
    ~A(){}
};

class Foo{
public:
    Foo(){}
    ~Foo(){}
    A& create(int random_arg){
         // create object A and return its reference
    }
};

void other_method(){
     Foo f;
     A a = f.create();
     // do stuff with a
{

I have considered three possible solutions:

1. create a raw pointer and return a reference, but this is bad because there is no guarantee that the object will be properly deleted:

   A& create(int random_arg){
        A* a = new A();
        return *a;
   }
   

2. create a shared_ptr and return the shared_ptr by value. In this way the shared_ptr will take care of the object deletion:

   shared_ptr<A> create(int random_arg){
        boost::shared_ptr<A> a_ptr(new A());
        return a_ptr;
   }
   

3. create a shared_ptr and return a reference:

   A& create(int random_arg){
        boost::shared_ptr<A> a_ptr(new A());
        return *a_ptr;
   }
   

The second solution seems to be the most used, but in this way I have to spread shared_ptr in the application and I would prefer to have references, or better const references.

What do you think is the best way to handle this situation? Are there other possibilities I have not considered?

Bushnell

Why do you want to do this? – Harbird

Why not just return by value and let RVO worry about it? – Tearjerker

This could end up being a bad idea. – Raquelraquela

(1) leads to memory leaks. (2) works but is unnecessary overhead and complexity. (3) is undefined behaviour if anyone ever looks at the return value. You didn't consider just A a; return a;. Follow the KISS principle. – Tiki

... I should add that I meant returning by value, not A& create() { A a; return a; } which has the same problem as (3) – Tiki

Answer 1

Don't do this. You're most likely to have a dangling reference.

Just return the instance of A by value instead.

It's likely that the compiler will elide the implied object copy. But you can guarantee that an object copy will not be made by writing a move constructor for A.

Lepido

Yes, the way to return an A is to return an A, and stop all the unnecessary complexity that leads to memory leaks or undefined behaviour. – Tiki

Thanks, it seems the easiest and safest way, but what if A has refrence to other objects as member variables? I'm not an expert about move constructor, does this mean that I should implement a deep copy of the object inside the move constructor to copy also the objects referenced by A? – Bushnell

It's not a good idea to have references as member variables since you'll always have to keep the referred objects in scope for the life of your instance. I'd advise (i) removing all such members and, (ii) allow the compiler to elide object copies where it can. Worry about optimisations at a later stage. Also, have a look at std::shared_ptr: it might be useful. – Lepido

Thanks, so you're suggestion is better to initialize class members passing to the constructor other objects by value and not by reference, am I right? – Bushnell

No, pass variables by constant reference to the constructor, but don't use reference types for member variables. – Lepido

Answer 2

To complete Bathsheba's answer:

There is only one case where you might want to do something like that, that is if you are returning an instance to a singleton. In this case such instance will be a static object:

A& getTheOnlyInstanceOfA()
{
   static A onlyInstOfA;
   return onlyInstOfA; 
}

Storm

Answer 3

The reason returning pointers or references to locals is a bad idea is the fact they are typically on a stack, which means the moment the stack reaches that depth again, the data will be overwritten and corrupted.

Newbies often make such mistakes, reassured by the fact it will often happen to work by pure luck, i.e. when you directly use the returned pointer before any other function has been called to corrupt the stack frame of the previous one.

Returning by value might be applicable in many cases, the compiler is free to optimize the return. But even without an optimization it might not be a big deal, for example in Qt all containers and many other classes do implicit sharing of resources, so returning by copy doesn't really copy all the data, just the shell of the object, usually a single pointer to the actual data. There is also move semantics in C++ which is sort of similar in that regard - it will move the responsibility from the local to the returned object without doing a deep copy.

There are two main cases where returning by value is not desired.

• if it involves an unavoidable copy of a heavy object
• if it is an object with "identity" - such have their copy constructor and assignment operators disabled, so they can't really be returned by value

In those cases you simply allocate the object dynamically and return a pointer to it. Whether you manage it manually or prefer to use a smart pointer - that's up to your needs.

Melbamelborn