402

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 worries 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.

artm
  • 17,291
  • 6
  • 38
  • 54
Nick Bolton
  • 38,276
  • 70
  • 174
  • 242
  • 1
    It is not evil if you are writing getter-like functions/methods. – John Z. Li Feb 10 '18 at 08:32
  • 1
    No, it is not. The user could always say whether he wants the reference (use &) or copy the reference (no &) so it is up to the user. It is in no way evil at all; but actually pretty convenient cause it gives more control and freedom to the developers, and also save memory. – Beyondo Jul 28 '22 at 22:54

16 Answers16

484

In general, returning a reference is perfectly normal and happens all the time.

If you mean:

int& getInt() {
    int i;
    return i;  // DON'T DO THIS.
}

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

int& getInt() {
    int* i = new int;
    return *i;  // DON'T DO THIS.
}

Because now the client has to eventually do the strange:

int& myInt = getInt(); // note the &, we cannot lose this reference!
delete &myInt;         // must delete...totally weird and  evil

int oops = getInt(); 
delete &oops; // undefined behavior, we're wrongly deleting a copy, not the original

Note that rvalue references are still just references, so all the evil applications remain the same.

If you want to allocate something that lives beyond the scope of the function, use a smart pointer (or in general, a container):

std::unique_ptr<int> getInt() {
    return std::make_unique<int>(0);
}

And now the client stores a smart pointer:

std::unique_ptr<int> x = getInt();

References are also okay for accessing things where you know the lifetime is being kept open on a higher-level, e.g.:

struct immutableint {
    immutableint(int i) : i_(i) {}

    const int& get() const { return i_; }
private:
    int i_;
};

Here we know it's okay to return a reference to i_ because whatever is calling us manages the lifetime of the class instance, so i_ will live at least that long.

And of course, there's nothing wrong with just:

int getInt() {
   return 0;
}

If the lifetime should be left up to the caller, and you're just computing the value.

Summary: it's okay to return a reference if the lifetime of the object won't end after the call.

Patrick B.
  • 11,773
  • 8
  • 58
  • 101
GManNickG
  • 494,350
  • 52
  • 494
  • 543
  • 29
    These are all bad examples. The best example of proper usage is when you are returning a reference to an object that was passed in. ala operator < – Arelius Jun 10 '11 at 21:46
  • 193
    For the sake of posterity, and for any newer programmers chancing upon this, **pointers are not bad**. Neither are pointers to dynamic memory bad. They both have their legitimate places in C++. Smart pointers should definitely be your default go-to when it comes to dynamic memory management though, but your default smart pointer should be unique_ptr, not shared_ptr. – Jamin Grey Mar 06 '13 at 20:22
  • 2
    The only time I find return by reference useful is while chaining function calls. Example: `Class& Class::translate(...) { ...; return *this}; Class& Class::rotate(...) { ...; return *this;}; Class obj; obj.translate(...).rotate(...).translate(...);`. – Jayesh Dec 06 '13 at 16:55
  • 16
    Edit approvers: don't approve edits if you cannot vouch for its correctness. I've rollbacked the incorrect edit. – GManNickG Mar 20 '14 at 04:01
  • 9
    For the sake of posterity, and for any newer programmers chancing upon this, **do not write `return new int`**. – Lightness Races in Orbit Apr 09 '15 at 19:57
  • 8
    For the sake of posterity, and for any newer programmers chancing upon this, just return the T from the function. RVO will take care of everything. – Shoe Apr 09 '15 at 19:59
  • int badInt = *getInt(); delete &badInt; // won't work. badInt was a copy of the allocated int, which // is now lost forever – Kaiserludi May 21 '15 at 12:27
  • The second case `int& getInt(void) { int *i = new int; return *i; }` is the most evil thing I have ever seen – UmNyobe Apr 22 '16 at 13:18
  • 1
    @Andrew Where are you selling potatoes? :D –  May 10 '17 at 21:01
  • 3
    Isn't the `immutableint` example dangerous as well, because the caller might hold on to the `int&` even after `immutableint` is destructed? Is this pattern recommended / advised against in common style guides? – bluenote10 Aug 20 '18 at 14:17
  • @bluenote10: It's not _immediately_ dangerous like the others, but it does open the possibility to use-after-free. However, this is a problem with references in general, not related to functions. It's usually much harder to have a reference outlive the thing it references in this fashion. It's fairly common: `operator[]` for example, almost universally returns a reference (or a proxy containing a reference), like in `std::vector`. – GManNickG Aug 20 '18 at 16:55
66

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.

Zitrax
  • 19,036
  • 20
  • 88
  • 110
Charlie Martin
  • 110,348
  • 25
  • 193
  • 263
  • 31
    I downvoted because this neither answers the question (in which OP has made clear he knows the need for deletion) nor addresses the legitimate fear that returning a reference to a freestore object can lead to confusion. Sigh. –  Apr 15 '09 at 16:58
  • 4
    The practice of returning a reference object is *not* evil. Ergo no. The fear he expresses is a correct fear, as I point out in the second graf. – Charlie Martin Apr 15 '09 at 17:01
  • You actually didnt. But this is not worth my time. –  Apr 15 '09 at 17:03
  • 3
    Iraimbilanja@ About the "No"-s I don't care. but this post pointed out an important info which was missing from GMan. – Kobor42 May 25 '13 at 20:17
59

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 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.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
David Thornley
  • 56,304
  • 9
  • 91
  • 158
  • 1
    Excellent answer but for "You can return a temporary as a const reference." The following code will compile but probably crash because the temporary is destroyed at the end of the return statement: "int const& f() { return 42; } void main() { int const& r = f(); ++r; }" – j_random_hacker Apr 16 '09 at 11:36
  • @j_random_hacker: C++ has some strange rules for references to temporaries, where the temporary lifetime might be extended. I'm sorry I don't understand it well enough to know if it covers your case. – Mark Ransom Apr 16 '09 at 16:41
  • 3
    @Mark: Yes, it does have some strange rules. A temporary's lifetime can only be extended by initialising a const reference (that is not a class member) with it; it then lives until the ref goes out of scope. Sadly, returning a const ref is *not* covered. Returning a temp by value is safe however. – j_random_hacker Apr 17 '09 at 03:56
  • See the C++ Standard, 12.2, paragraph 5. Also see Herb Sutter's stray Guru of the Week at http://herbsutter.wordpress.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/. – David Thornley Apr 17 '09 at 13:27
  • @David: At first that's how I interpreted it too, but in fact that paragraph, and Sutter's example, don't apply to a function whose return type is T const& -- the function must return a T **by value**, which is then used to initialise a variable of type T const& for lifetime extension to happen. – j_random_hacker Apr 18 '09 at 16:22
  • 4
    @David: When the function's return type is "T const&", what actually happens is that the return statement **implicitly converts** the temp, which is of type T, to type "T const&" as per 6.6.3.2 (a legal conversion but one which does not extend lifetime), and then the calling code initialises the ref of type "T const&" with the function's result, also of type "T const&" -- again, a legal but non-lifetime-extending process. End result: no extension of lifetime, and much confusion. :( – j_random_hacker Apr 18 '09 at 16:28
45

I find the answers not satisfactory so I'll add my two cents.

Let's analyze the following cases:

Erroneous usage

int& getInt()
{
    int x = 4;
    return x;
}

This is obviously error

int& x = getInt(); // will refer to garbage

Usage with static variables

int& getInt()
{
   static int x = 4;
   return x;
}

This is right, because static variables are existant throughout lifetime of a program.

int& x = getInt(); // valid reference, x = 4

This is also quite common when implementing Singleton pattern

class Singleton
{
    public:
        static Singleton& instance()
        {
            static Singleton instance;
            return instance;
        };

        void printHello()
        {
             printf("Hello");
        };

};

Usage:

 Singleton& my_sing = Singleton::instance(); // Valid Singleton instance
 my_sing.printHello();  // "Hello"
    

Operators

Standard library containers depend heavily upon usage of operators which return reference, for example

T & operator*();

may be used in the following

std::vector<int> x = {1, 2, 3}; // create vector with 3 elements
std::vector<int>::iterator iter = x.begin(); // iterator points to first element (1)
*iter = 2; // modify first element, x = {2, 2, 3} now

Quick access to internal data

There are times when & may be used for quick access to internal data

Class Container
{
    private:
        std::vector<int> m_data;

    public:
        std::vector<int>& data()
        {
             return m_data;
        }
}

with usage:

Container cont;
cont.data().push_back(1); // appends element to std::vector<int>
cont.data()[0] // 1

HOWEVER, this may lead to pitfall such as this:

Container* cont = new Container;
std::vector<int>& cont_data = cont->data();
cont_data.push_back(1);
delete cont; // This is bad, because we still have a dangling reference to its internal data!
cont_data[0]; // dangling reference!
Daniel K.
  • 919
  • 10
  • 17
thorhunter
  • 483
  • 7
  • 9
  • Returning the reference to a static variable can lead to undesired behavior e.g. consider a multiply operator which returns a reference to a static member then the following will always result to `true`: `If((a*b) == (c*d))` – clickMe Dec 21 '16 at 20:55
  • `Container::data()`'s implementation should read `return m_data;` – Xeaz Feb 21 '17 at 07:31
  • This was very helpful, thanks! @Xeaz wouldn't that cause issues with the append call though? – Andrew Feb 26 '17 at 05:54
  • @Andrew No, it was a syntax shenanigan. If you, for example, returned a pointer type, then you would use reference address to create and return a pointer. – thorhunter Feb 27 '17 at 10:23
16

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")

Mehrdad Afshari
  • 414,610
  • 91
  • 852
  • 789
  • 1
    I'm just curious, what is good about `map[name] = "hello world"`? – wrongusername Oct 20 '11 at 07:03
  • 7
    @wrongusername The syntax is intuitive. Ever tried to increment the count of a value stored in a `HashMap` in Java? :P – Mehrdad Afshari Oct 20 '11 at 07:29
  • Haha, not yet, but looking at HashMap examples, it does look quite gnarly :D – wrongusername Oct 20 '11 at 16:45
  • Problem I had with this: Function returns reference to an object in a container, but the calling function code assigned it to a local variable. Then modified some properties of the object. Problem: Original object in the container stayed untouched. The programmer so easily overlooks the & in the return value, and then you get really unexpected behaviours... – flohack Jun 10 '14 at 16:28
11

"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.

Community
  • 1
  • 1
John Dibling
  • 99,718
  • 31
  • 186
  • 324
8

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.

  • 1
    non-const ref doesnt necessarily break encapsulation. consider vector::operator[] –  Apr 15 '09 at 17:02
  • that's a very special case... that's why I said sometimes, though I should really claim MOST OF THE TIME :) –  Apr 15 '09 at 17:04
  • So, you're saying that the normal subscript operator implementation a necessary evil? I neither disagree or agree with this; as I am none the wiser. – Nick Bolton Apr 15 '09 at 17:06
  • I don't say that, but it can be evil if misused :))) vector::at should be used whenever possible.... –  Apr 15 '09 at 17:07
  • eh? vector::at also returns a nonconst ref. –  Apr 15 '09 at 17:18
  • `vector::at` does bounds checking, don't use it unless you know you have to. – Miles Rout Oct 29 '14 at 04:24
  • ^ irrelevant, and anyway, i'd say _do_ use it unless you know you _don't_ have to (e.g, in the class internals where you're safe from bad user input) – underscore_d Dec 06 '15 at 17:55
5

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.

  • Ah, yes of course; I think this is why I started using it; as when I first implemented the subscript operator [] I realized the use of references. I'm lead to believe that this is the de facto. – Nick Bolton Apr 15 '09 at 17:03
  • 1
    Oddly enough, you can implement `operator[]` for a container without using a reference... and `std::vector` does. (And creates a real mess in the process) – Ben Voigt Jul 31 '17 at 19:36
  • @BenVoigt mmm, why a mess? Returning a proxy is also a valid scenario for containers with complex storage that does not map directly to the outer types (like `::std::vector` you've mentioned). – Sergey.quixoticaxis.Ivanov Oct 12 '17 at 18:31
  • 1
    @Sergey.quixoticaxis.Ivanov: The mess is that using `std::vector` in template code is broken, if `T` might be `bool`, because `std::vector` has very different behavior from other instantiations. It's useful, but it should have been given its own name and not a specialization of `std::vector`. – Ben Voigt Oct 12 '17 at 18:36
  • @BenVoight I agree on the point about strange decision of making one specialization "really special" but I felt that your original comment implies that returning a proxy is odd in general. – Sergey.quixoticaxis.Ivanov Oct 12 '17 at 18:42
2

Addition to the accepted answer:

struct immutableint {
    immutableint(int i) : i_(i) {}

    const int& get() const { return i_; }
private:
    int i_;
};

I'd argue that this example is not okay and should be avoided if possible. Why? It is very easy to end-up with a dangling reference.

To illustrate the point with an example:

struct Foo
{
    Foo(int i = 42) : boo_(i) {}
    immutableint boo()
    {
        return boo_;
    }  
private:
    immutableint boo_;
};

entering the danger-zone:

Foo foo;
const int& dangling = foo.boo().get(); // dangling reference!
AMA
  • 4,114
  • 18
  • 32
1

return reference is usually used in operator overloading in C++ for large Object, because returning a value need copy operation.(in perator overloading, we usually don't use pointer as return value)

But return reference may cause memory allocation problem. Because a reference to the result will be passed out of the function as a reference to the return value, the return value cannot be an automatic variable.

if you want use returning refernce, you may use a buffer of static object. for example

const max_tmp=5; 
Obj& get_tmp()
{
 static int buf=0;
 static Obj Buf[max_tmp];
  if(buf==max_tmp) buf=0;
  return Buf[buf++];
}
Obj& operator+(const Obj& o1, const Obj& o1)
{
 Obj& res=get_tmp();
 // +operation
  return res;
 }

in this way, you could use returning reference safely.

But you could always use pointer instead of reference for returning value in functiong.

MinandLucy
  • 76
  • 4
0

i think using reference as the return value of the function is much more straight forward than using pointer as the return value of the function. Secondly It would be always safe to use static variable to which the return value refer to.

Tony
  • 89
  • 10
0

Best thing is to create object and pass it as reference/pointer parameter to a function which allocates this variable.

Allocating object in function and returning it as a reference or pointer (pointer is safer however) is bad idea because of freeing memory at the end of function block.

Abhinav Singh Maurya
  • 3,313
  • 8
  • 33
  • 51
Drezir
  • 1
-1
    Class Set {
    int *ptr;
    int size;

    public: 
    Set(){
     size =0;
         }

     Set(int size) {
      this->size = size;
      ptr = new int [size];
     }

    int& getPtr(int i) {
     return ptr[i];  // bad practice 
     }
  };

getPtr function can access dynamic memory after deletion or even a null object. Which can cause Bad Access Exceptions. Instead getter and setter should be implemented and size verified before returning.

Amarbir
  • 23
  • 6
-2

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.

Dan Olson
  • 22,849
  • 4
  • 42
  • 56
  • 2
    Sure it's dangerous, and there should be better compiler error checking, but without it some useful constructs could not be done, e.g. operator[]() in std::map. – j_random_hacker Apr 16 '09 at 11:38
  • 2
    Returning non-const references is actually incredibly useful. `vector::operator[]` for example. Would you rather write `v.setAt(i, x)` or `v[i] = x`? The latter is FAR superior. – Miles Rout Oct 29 '14 at 04:26
  • 1
    @MilesRout I'd go for `v.setAt(i, x)` any time. It is FAR superior. – scravy Aug 01 '18 at 10:56
-3

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

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
developresource
  • 422
  • 4
  • 8
  • 2
    If you need to modify the original value in the calling code, then you *need* to return a ref. And that is in fact no more and no less dangerous than returning an iterator to a vector -- both are invalidated if elements are added to or removed from the vector. – j_random_hacker Apr 16 '09 at 11:43
  • 1
    That particular problem was caused by holding a reference to a vector element, and then modifying that vector in a way which invalidates the reference: Page 153, section 6.2 of "C++ Standard Library: A Tutorial and Reference" - Josuttis, reads: "Inserting or removing elements invalidates references, pointers, and iterators that refer to the following elements. If an insertion causes reallocation, it invalidates all references, iterators, and pointers" – Trent Feb 19 '13 at 00:58
-16

About horrible code:

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

So, indeed, memory pointer lost after return. But if you use shared_ptr like that:

int& getTheValue()
{
   std::shared_ptr<int> p(new int);
   return *p->get();
}

Memory not lost after return and will be freed after assignment.

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
Anatoly
  • 23
  • 1