Is the practice of returning a C++ reference variable, evil?

Question

This is a little subjective I think; I'm not sure if the opinion will be unanimous (I've seen a lot of code snippets where references are returned).

According to a comment toward this question I just asked, regarding initializing references, returning a reference can be evil because, [as I understand] it makes it easier to miss deleting it, which can lead to memory leaks.

This worrys me, as I have followed examples (unless I'm imagining things) and done this in a fair few places... Have I misunderstood? Is it evil? If so, just how evil?

I feel that because of my mixed bag of pointers and references, combined with the fact that I'm new to C++, and total confusion over what to use when, my applications must be memory leak hell...

Also, I understand that using smart/shared pointers is generally accepted as the best way to avoid memory leaks, so thanks in advance for your tips ;)

Answer 1

It's not evil. Like many things in C++, it's good if used correctly, but there are many pitfalls you should be aware of when using it (like returning a reference to a local variable).

There are good things that can be achieved with it (like map[name] = "hello world")

Answer 2

No. No, no, a thousand times no.

What is evil is making a reference to a dynamically allocated object and losing the original pointer. When you new an object you assume an obligation to have a guaranteed delete.

But have a look at, eg, operator<<: that must return a reference, or

cout << "foo" << "bar" << "bletch" << endl ;

won't work.

Answer 3

Not only is it not evil, it is sometimes essential. For example, it would be impossible to implement the [] operator of std::vector without using a reference return value.

Answer 4

Do you mean:

int& getInt(void)
{
    int i;
    return i;
}

That is all sorts of evil. The stack-allocated i will go away and you are referring to nothing. This is semi-evil:

int& getInt(void)
{
    int *i = new int;
    return *i;
}

Because now the client has to eventually do the strange:

int& myInt = getInt(); // note the &.
int badInt = getInt(); // the & will be easy to miss (source of problems).
delete &myInt; // must delete.
delete &badInt; // won't work. badInt was a copy of the allocated int, which
                // is now lost forever

I think the best way to do something like that is just:

int *getInt(void)
{
    return new int;
}

And now the client stores a pointer:

int *myInt = getInt(); // has to be a pointer
int& weirdInt = *getInt(); // but this works too if you really want.
delete myInt; // being a pointer, this is easy to do.
delete &weirdInt; // works.

Now for members of classes, & is powerful, such as operator chaining (cout's <<), or operator[].

Answer 5

There are two cases:

• const reference --good idea, sometimes, especially for heavy objects or proxy classes, compiler optimization

• non-const reference --bad idea, sometimes, breaks encapsulations

Both share same issue -- can potentially point to destroyed object...

I would recommend using smart pointers for many situations where you require to return a reference/pointer.

Also, note the following:

There is a formal rule - the C++ Standard (section 13.3.3.1.4 if you are interested) states that a temporary can only be bound to a const reference - if you try to use a non-const reference the compiler must flag this as an error.

Answer 6

"returning a reference is evil because, simply [as I understand] it makes it easier to miss deleting it"

Not true. Returning a reference does not imply ownership semantics. That is, just because you do this:

Value& v = thing->getTheValue();

...does not mean you now own the memory referred to by v;

However, this is horrible code:

int& getTheValue()
{
   return *new int;
}

If you are doing something like this because "you don't require a pointer on that instance" then: 1) just dereference the pointer if you need a reference, and 2) you will eventually need the pointer, because you have to match a new with a delete, and you need a pointer to call delete.

Answer 7

You should return a reference to an existing object that isn't going away immediately, and where you don't intend any transfer of ownership.

Never return a reference to a local variable or some such, because it won't be there to be referenced.

You can return a temporary as a const reference. A temporary is something like a + b, where the compiler creates a temporary object that will normally go away at the end of the expression. You might implement operator+ like that.

You can return a reference to something independent of the function, which you don't expect the calling function to take the responsibility for deleting. This is the case for the typical operator[] function.

If you are creating something, you should return either a value or a pointer (regular or smart). You can return a value freely, since it's going into a variable or expression in the calling function. Never return a pointer to a local variable, since it will go away.

Answer 8

Function as lvalue (aka, returning of non-const references) should be removed from C++. It's terribly unintuitive. Scott Meyers wanted a min() with this behavior.

min(a,b) = 0;  // What???

which isn't really an improvement on

setmin (a, b, 0);

The latter even makes more sense.

I realize that function as lvalue is important for C++ style streams, but it's worth pointing out that C++ style streams are terrible. I'm not the only one that thinks this... as I recall Alexandrescu had a large article on how to do better, and I believe boost has also tried to create a better type safe I/O method.

Answer 9

I ran into a real problem where it was indeed evil. Essentially a developer returned a reference to an object in a vector. That was Bad!!!

The full details I wrote about in Janurary: http://developer-resource.blogspot.com/2009/01/pros-and-cons-of-returing-references.html

Hope that helps, @developresource