19

I've tried to Google this issue, and I can't find anything that I see as relevant. So I must be looking for the wrong thing; none the less, I'd appreciate some advice...

Foobar &foobar = *new Foobar(someArg, anotherArg);

Is it just me or does that seem smelly?

I understand that the new keyword is designed for use with pointers (as such):

Foobar *foobar = new Foobar(someArg, anotherArg);

But what if you don't require a pointer on that instance, and you would like to use a reference instead? Or, is it the case that you don't need to explicitly initialize it (much like local variables); and if this is the case, what if I want to initialize with parameters?

The following does not work (unless I'm doing something wrong):

// the last argument is of type: int
Foobar &foobar(someArg, anotherArg);

... gives the compiler error:

initializer expression list treated as compound expression invalid initialization of non-const reference of type ‘Foobar&’ from a temporary of type ‘int’

And also this doesn't seem to work:

Foobar &foobar = Foobar(someArg, anotherArg);

... gives the compiler error:

error: invalid initialization of non-const reference of type ‘Foobar&’ from a temporary of type ‘Foobar’

Update 1:

Bare in mind that I am returning this value, so I don't want to use a local variable; I want to use a value on the heap, not the stack:

Foobar &helloWorld()
{
    Foobar &foobar = *new Foobar(someArg, anotherArg);
    foobar.HelloWorld();
    return foobar;
}

Should I just be using pointers instead, or is this completely valid?

Agostino
  • 2,723
  • 9
  • 48
  • 65
Nick Bolton
  • 38,276
  • 70
  • 174
  • 242
  • 1
    It works... but as I say in my answer, that's unconventional, so no other programmer using helloWorld will expect they have to delete the returned Foobar, and it makes it very memory leak prone. – Scott Langham Apr 15 '09 at 16:28
  • You are better off returning a pointer. Or better still, see the stl::auto_pointer. Return one of those, and whatever the programmer using your function does with the result, it will be guaranteed to be deleted again without leaving a leak. – Scott Langham Apr 15 '09 at 16:29
  • Great tip for auto_ptr! :) I will start using that instead of returning references... – Nick Bolton Apr 15 '09 at 16:36
  • r3n: that is so evil. How will you ever know when to free Foobar & >:-). I'm sure you can write that so it doesnt use new. Also, if you look at c++0x you'll find the && operator. In a year when it comes out doing Foobar fooVar(someArg, anotherArg); return fooVar; is efficient. –  Apr 15 '09 at 16:40
  • acidzombie24: Haha, yes, I never thought about how references are misleading... Maybe I'm evil by nature; oh dear! – Nick Bolton Apr 15 '09 at 16:42
  • r3n: PS auto_ptr is evil (at least, bad. c++0x is getting rid of it IIRC) http://www.gotw.ca/gotw/042.htm http://www.gotw.ca/gotw/025.htm –  Apr 15 '09 at 16:43
  • lol, r3n you use to do C#? i do a lot of c++ and moving into C#. Maybe we should exchange IM and help eachother out. email me at gmail.com if you like –  Apr 15 '09 at 16:45
  • yep. I think auto_ptr will be replaced by unique_ptr, but for now I think it's a good option. Or, shared_ptr would probably do the job and hopefully won't be deprecated too soon. – Scott Langham Apr 15 '09 at 16:50
  • @acidzombie24: Yes indeed, always looking for like minded programmers, not sure how we will exchange details, I don't want to post my IM ID publically. By the way, this will interest you: http://stackoverflow.com/questions/752658/is-the-practice-of-returning-a-c-reference-variable-evil – Nick Bolton Apr 15 '09 at 16:51
  • lol it actually doesnt. I have done enough crazy code in c++ that i dont have problems passing data around. –  Apr 15 '09 at 17:29
  • ... oh well haha, what was I thinking? ;) – Nick Bolton Apr 15 '09 at 18:03

6 Answers6

11

Why do you think you need to use new and references at all? Why not:

Foobar foobar(someArg, anotherArg);

For your function - return a value:

Foobar helloWorld()
{
    Foobar foobar(someArg, anotherArg);
    foobar.HelloWorld();
    return foobar;
}

or a pointer:

Foobar * helloWorld()
{
    Foobar * foobar = new Foobar(someArg, anotherArg);
    foobar->HelloWorld();
    return foobar;
}

If you do this - the caller is responsible for deleting the allocated object at some point.

Return from a non-member function is one place where references can typically not be used sensibly, as the thing you would like to refer to usually no longer exists.

  • Ah, I was avoiding the method described in the 2nd snippet because I assumed it produced a stack assigned memory value... Previously I was using this, and getting SIGSEGV errors; after changing to returning pointers or references, the seg faults went away... – Nick Bolton Apr 15 '09 at 16:34
  • 1
    By second method, I mean the method: Foobar helloWorld() – Nick Bolton Apr 15 '09 at 16:37
  • The value return requires aworking copy constructor and destructor - if you were getting memory access errors, one of these (or both) is probably not implemented correctly. –  Apr 15 '09 at 16:42
  • +1. The by-value approach in Neil's 2nd snippet is by far the clearest and least error prone, provided Foobar is a value-like type and not too enormous. – j_random_hacker Apr 15 '09 at 16:58
  • 1
    Just to clarify, only the pointer version leaves the caller responsible for deleting, right? In the value return version the compiler will remove the copied value from the stack when it goes out of scope of the caller. – Chinasaur Sep 19 '11 at 21:09
6

Yes, that is smelly!

If you 'new' something, assign it to a pointer (or smart-pointer type), as it will need to be deleted again to avoid a memory leak. References aren't conventionally thought of as being things you need to delete again, so if somebody else sees that code (assigning a new'ed object to a reference), it may well confuse them.

You can do...

const Foobar &foobar = Foobar(someArg, anotherArg);

...if you really want a reference. Note that once foobar goes out of scope, the temporary object it is referencing will die. But, there's not a lot of point in writing that when you can straight-forwardly write:

Foobar foobar(someArg, anotherArg);

You probably don't actually need a reference... they're generally (but not exclusively) used for the types of method arguments. This is so that you can pass something that looks like an object, but only has the size of a pointer, and which the method can modify. The reference was introduced primarily to allow you to write a copy constructor (I won't explain that here!).

Scott Langham
  • 58,735
  • 39
  • 131
  • 204
  • 1
    Hmm, are you sure your 1st snippet is correct? This seems to work fine for class-wide references, but not for in line declarations; I get this error... invalid initialization of non-const reference of type ‘MyType&’ from a temporary of type ‘MyType’ – Nick Bolton Apr 15 '09 at 16:29
  • r3n thats correct. const Foobar& foobar would work. Typically you dont use reference for temp objs, you pass temp obj as a whole and receive them as a whole (instead of as a pointer/ref) –  Apr 15 '09 at 16:35
  • I'm pretty sure I read somewhere that this works. I'm afraid I can't remember where. It does seem to compile for me using MS Visual Studio 2008. – Scott Langham Apr 15 '09 at 16:37
  • class MyType { public: MyType(int v) : j(v) {} int j; }; int _tmain(int argc, _TCHAR* argv[]) { MyType& mine = MyType(3); – Scott Langham Apr 15 '09 at 16:38
  • 1
    Scott: Yes, this is a known MSVC bug. –  Apr 15 '09 at 16:40
  • 1
    Looks like you're right http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=199 – Scott Langham Apr 15 '09 at 16:47
  • 1
    +1, but please mention that the temporary bound to your const ref in the 1st snippet will be destroyed at the end of the function, meaning that it's useless to return a reference to it -- any code that uses the reference will likely crash. – j_random_hacker Apr 15 '09 at 17:04
3

References in C++ are really not as strong as people expect them to be, I think the confusion comes from people who are used to languages like Java and C# that don't have pointers and have references that can be reassigned and used.

A reference in C++ is generally best used as an alias for a variable, so you can simplify things like parameter passing and return values. There are very few situations where you would try to acquire a reference the way you are doing on the first line. So usually you don't need to do what it seems that you're trying to do :)

The second line is of course correct, and you could do something like return *foobar from a function that returns a reference.

Uri
  • 88,451
  • 51
  • 221
  • 321
  • Hmm, so you would recommend [in general] that I return pointers because they're more robust? – Nick Bolton Apr 15 '09 at 16:27
  • It's ok to return references in situations where they are appropriate, like operators, so you will often see a return by reference. – Uri Apr 15 '09 at 16:37
2

Your writing it right. But keep in mind its weird to free the ptr by doing delete &refVar; (it could be mistaken for a variable that was not created by new).

You should check out GotW for good practices passing around http://www.gotw.ca/gotw/ I dont remember which lesson it was (there was more then one) but reading through that is more valuable then anyone realize (gotchas will be less of a surprise)

You should try writing code that NEVER use pointers. I do it but once got stuck when i needed a container of BaseObj. There was no working around that. Typically i used the STL containers and have most variable alive on the stack { MyClass myVar; ... } or as a member and pass it around as needed.

Its quiet easy to remove pointers once you start passing reference instead of pointers and use stl containers instead of new. Note that i never use auto_ptr. vector, string, deque, list and map should do most of what you need. There are other containers.

1

Nope, you've got it. A foo& "points to" the actual object, so if you really want a foo reference you have to dereference the foo pointer.

@Neil has a point, though -- syntactically that's how you get what you want, but why do you want it?

Charlie Martin
  • 110,348
  • 25
  • 193
  • 263
1

I'd recommend using a smart pointer. You need to manage ownership, and a reference will not do that for you. In fact, it will obscure to the caller that there's any ownership issue at all. As Scott Langham mentioned in a comment, std::auto_ptr would work. Using shared_ptr from Boost or TR1 might be a better idea.

boost::shared_ptr<Foobar> helloWorld()
{
    boost::shared_ptr<Foobar> foobar(new Foobar(someArg, anotherArg));
    foobar->HelloWorld();
    return foobar;
}
Fred Larson
  • 60,987
  • 18
  • 112
  • 174