views:

418

answers:

7

Hello, I know the title sounds familiar as there are many similar questions, but I'm asking for a different aspect of the problem (I know the difference between having things on the stack and putting them on the heap).

In Java I can always return references to "local" objects

public Thing calculateThing() {
    Thing thing = new Thing();
    // do calculations and modify thing
    return thing;
}

In C++, to do something similar I have 2 options

(1) I can use references whenever I need to "return" an object

void calculateThing(Thing& thing) {
    // do calculations and modify thing
}

Then use it like this

Thing thing;
calculateThing(thing);

(2) Or I can return a pointer to a dynamically allocated object

Thing* calculateThing() {
    Thing* thing(new Thing());
    // do calculations and modify thing
    return thing;
}

Then use it like this

Thing* thing = calculateThing();
delete thing;

Using the first approach I won't have to free memory manually, but to me it makes the code difficult to read. The problem with the second approach is, I'll have to remember to delete thing;, which doesn't look quite nice. I don't want to return a copied value because it's inefficient (I think), so here come the questions

  • Is there a third solution (that doesn't require copying the value)?
  • Is there any problem if I stick to the first solution?
  • When and why should I use the second solution?
+25  A: 

I don't want to return a copied value because it's inefficient

Prove it.

Look up RVO and NRVO, and in C++0x move-semantics. In most cases in C++03, an out parameter is just a good way to make your code ugly, and in C++0x you'd actually be hurting yourself by using an out parameter.

Just write clean code, return by value. If performance is a problem, profile it (stop guessing), and find what you can do to fix it. It likely won't be returning things from functions.


That said, if you're dead set on writing like that, you'd probably want to do the out parameter. It avoids dynamic memory allocation, which is safer and generally faster. It does require you have some way to construct the object prior to calling the function, which doesn't always make sense for all objects.

If you want to use dynamic allocation, the least that can be done is put it in a smart pointer. (This should be done all the time anyway) Then you don't worry about deleting anything, things are exception-safe, etc. The only problem is it's likely slower than returning by value anyway!

GMan
Thanks GMan, I'm just studying C++ and I read somewhere that when returning a value, the value will have to be copied to somewhere else and the copying looks inefficient, isn't it?
phunehehe
@phunehehe: No point is speculating, you should profile your code and find out. (Hint: no.) Compilers are very smart, they aren't going to waste time copying things around if they don't have to. *Even if* copying cost something, you should still strive for good code over fast code; good code is easy to optimize when speed becomes a problem. No point in uglying up code for something you have no idea is a problem; especially if you actually slow it down or get nothing out of it. And if you're using C++0x, move-semantics make this a non-issue.
GMan
@phunehehe this may also be useful in some cases, http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/
aaa
@GMan, re: RVO: actually this is only true if your caller and callee happen to be in the same compilation unit, which in real world it isn't most of the time. So, you are in for disappointment if your code isn't all templated (in which case it will all be in one compilation unit) or you have some link-time optimization in place (GCC only has it from 4.5).
Alex B
@Alex: Compilers are getting better and better at optimizing across translation units. (VC does it for several releases now.)
sbi
@GMan: +1 Very nice answer. Do you have any link/article that would explain the "move-semantics" more deeply ?
ereOn
Excellent answer, `+1` from me. Add to this the fact that returning by value saves the cost of dynamic memory allocation/deallocation, which might actually _speed up_ the code.
sbi
@Alex B: This is complete garbage. Many very common calling conventions make the caller responsible for allocating space for large return values and the callee responsible for their construction. RVO happily works across compilation units even without link time optimizations.
Charles Bailey
@ereOn: Hm, there are surprisingly little articles online about move-semantics! Maybe [this](http://www.devx.com/cplus/10MinuteSolution/34577) can help. Basically C++0x provides a way for classes to interact with rvalues (which temporaries are, and lvalues can be made into rvalues). And rvalues are values not intended to keep their resources around (either because they are temporary, or you used `std::move` on an lvalue), so it's safe to simply take their resources instead of copy them. So by providing a way to tell the compiler how to move resources around with consideration to your...
GMan
...classes, you can eliminate all needless copying. When returning, an object is "moved" from inside the function to outside, rather than copied. I give a real example of move-semantics (and even do a step-by-step optimization example) in my [copy-and-swap answer](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). For a high level way of thinking about the concept: Copying says, "I'm copying your resources, you keep yours", moving says "I'm taking your resources, and you have none.". Moving is extremely simple (check out the code in the answer I linked).
GMan
@Charles, upon checking it appears to be correct! I withdraw my clearly misinformed statement.
Alex B
A: 

I'm sure a C++ expert will come along with a better answer, but personally I like the second approach. Using smart pointers helps with the problem of forgetting to delete and as you say, it looks cleaner than having to create an object before hand (and still having to delete it if you want to allocate it on the heap).

Evgeny
+1  A: 

Firstly you have an error in the code, you mean to have Thing *thing(new Thing());, and only return thing;.

  • Use shared_ptr<Thing>. Deref it as tho it was a pointer. It will be deleted for you when the last reference to the Thing contained goes out of scope.
  • The first solution is very common in naive libraries. It has some performance, and syntactical overhead, avoid it if possible
  • Use the second solution only if you can guarantee no exceptions will be thrown, or when performance is absolutely critical (you will be interfacing with C or assembly before this even becomes relevant).
Matt Joiner
thanks Matt, I've fix the syntax
phunehehe
+2  A: 

Just create the object and return it

Thing calculateThing() {
    Thing thing();
    // do calculations and modify thing
     return thing;
}

I think you'll do yourself a favor if you forget about optimization and just write readable code (you'll need to run a profiler later - but don't pre-optimize).

Amir Rachum
`Thing thing();` declares a local function and returning a `Thing`.
dreamlax
Thing thing() declares a function returning a thing. There is no Thing object constructed in your function body.
Charles Bailey
@dream @Charles: Hivemind? :)
GMan
+6  A: 

Did you try to use smart pointers (if Thing is really big and heavy object), like auto_ptr:


std::auto_ptr<Thing> calculateThing()
{
  std::auto_ptr<Thing> thing(new Thing);
  // .. some calculations
  return thing;
}


// ...
{
  std::auto_ptr<Thing> thing = calculateThing();
  // working with thing

  // auto_ptr frees thing 
}
demetrios
+1  A: 

One quick way to determine if a copy constructor is being called is to add logging to your class's copy constructor:

MyClass::MyClass(const MyClass &other)
{
    std::cout << "Copy constructor was called" << std::endl;
}

MyClass someFunction()
{
    MyClass dummy;
    return dummy;
}

Call someFunction; the number of "Copy constructor was called" lines that you will get will vary between 0, 1, and 2. If you get none, then your compiler has optimised the return value out (which it is allowed to do). If you get don't get 0, and your copy constructor is ridiculously expensive, then search for alternative ways to return instances from your functions.

dreamlax
+1  A: 

Just return a object like this:

Thing calculateThing() 
{
   Thing thing();
   // do calculations and modify thing
   return thing;
}

This will invoke the copy constructor on Things, so you might want to do your own implementation of that. Like this:

Thing(const Thing& aThing) {}

This might perform a little slower, but it might not be an issue at all.

Update

The compiler will probably optimize the call to the copy constructor, so there will bee no extra overhead. (Like dreamlax pointed out in the comment).

Martin Ingvar Kofoed Jensen
`Thing thing();` declares a local function returning a `Thing`, also, the standard permits the compiler to omit the copy constructor in the case you presented; any modern compiler will probably do it.
dreamlax
@dreamlax: Agreed
Martin Ingvar Kofoed Jensen