C++ best practice: Returning reference vs. object

Question

Hi folks,

I'm trying to learn C++, and trying to understand returning objects. I seem to see 2 ways of doing this, and need to understand what is the best practice.

Option 1:

QList<Weight *> ret;
Weight *weight = new Weight(cname, "Weight");
ret.append(weight);
ret.append(c);
return &ret;

Option 2:

QList<Weight *> *ret = new QList();
Weight *weight = new Weight(cname, "Weight");
ret->append(weight);
ret->append(c);
return ret;

(of course, I may not understand this yet either).

Which way is considered best-practice, and should be followed?

TIA

Mike

Answer 1

Use the option #1 with a slight change; instead of returning a reference to the locally created object, return its copy.

i.e. return ret;

Most C++ compilers perform Return value optimization (RVO) to optimize away the temporary object created to hold a function's return value.

Answer 2

Option 1 is defective. When you declare an object

QList<Weight *> ret;

it only lives in the local scope. It is destroyed when the function exits. However, you can make this work with

return ret; // no "&"

Now, although ret is destroyed, a copy is made first and passed back to the caller.

This is the generally preferred methodology. In fact, the copy-and-destroy operation (which accomplishes nothing, really) is usually elided, or optimized out and you get a fast, elegant program.

Option 2 works, but then you have a pointer to the heap. One way of looking at C++ is that the purpose of the language is to avoid manual memory management such as that. Sometimes you do want to manage objects on the heap, but option 1 still allows that:

QList<Weight *> *myList = new QList<Weight *>( getWeights() );

where getWeights is your example function. (In this case, you may have to define a copy constructor QList::QList( QList const & ), but like the previous example, it will probably not get called.)

Likewise, you probably should avoid having a list of pointers. The list should store the objects directly. Try using std::list… practice with the language features is more important than practice implementing data structures.

Answer 3

In general, you should never return a reference or a pointer. Instead, return a copy of the object or return a smart pointer class which owns the object. In general, use static storage allocation unless the size varies at runtime or the lifetime of the object requires that it be allocated using dynamic storage allocation.

As has been pointed out, your example of returning by reference returns a reference to an object that no longer exists (since it has gone out of scope) and hence are invoking undefined behavior. This is the reason you should never return a reference. You should never return a raw pointer, because ownership is unclear.

It should also be noted that returning by value is incredibly cheap due to return-value optimization (RVO), and will soon be even cheaper due to the introduction of rvalue references.

Answer 4

passing & returning references invites responsibilty.! u need to take care that when you modify some values there are no side effects. same in the case of pointers. I reccomend you to retun objects. (BUT IT VERY-MUCH DEPENDS ON WHAT EXACTLY YOU WANT TO DO)

In ur Option 1, you return the address and Thats VERY bad as this could lead to undefined behaviour. (ret will be deallocated, but y'll access ret's address in the called function)

so use return ret;

Answer 5

It's generally bad practice to allocate memory that has to be freed elsewhere. That's one of the reasons we have C++ rather than just C. (But savvy programmers were writing object-oriented code in C long before the Age of Stroustrup.) Well-constructed objects have quick copy and assignment operators (sometimes using reference-counting), and they automatically free up the memory that they "own" when they are freed and their DTOR automatically is called. So you can toss them around cheerfully, rather than using pointers to them.

Therefore, depending on what you want to do, the best practice is very likely "none of the above." Whenever you are tempted to use "new" anywhere other than in a CTOR, think about it. Probably you don't want to use "new" at all. If you do, the resulting pointer should probably be wrapped in some kind of smart pointer. You can go for weeks and months without ever calling "new", because the "new" and "delete" are taken care of in standard classes or class templates like std::list and std::vector.

One exception is when you are using an old fashion library like OpenCV that sometimes requires that you create a new object, and hand off a pointer to it to the system, which takes ownership.

If QList and Weight are properly written to clean up after themselves in their DTORS, what you want is,

QList<Weight> ret();
Weight weight(cname, "Weight");
ret.append(weight);
ret.append(c);
return ret;

Answer 6

As already mentioned, it's better to avoid allocating memory which must be deallocated elsewhere. This is what I prefer doing (...these days):

void someFunc(QList<Weight *>& list){
    // ... other code
    Weight *weight = new Weight(cname, "Weight");
    list->append(weight);
    list->append(c);
}

// ... later ...

QList<Weight *> list;
someFunc(list)

Even better -- avoid new completely and using std::vector:

void someFunc(std::vector<Weight>& list){
    // ... other code
    Weight weight(cname, "Weight");
    list.push_back(weight);
    list.push_back(c);
}

// ... later ...

std::vector<Weight> list;
someFunc(list);

You can always use a bool or enum if you want to return a status flag.