views:

938

answers:

9

What is the best practice when returning a smart pointer, for example a boost::shared_ptr? Should I by standard return the smart pointer, or the underlying raw pointer? I come from C# so I tend to always return smart pointers, because it feels right. Like this (skipping const-correctness for shorter code):

class X
{
public:
    boost::shared_ptr<Y> getInternal() {return m_internal;}

private:
    boost::shared_ptr<Y> m_internal;
}

However I've seen some experienced coders returning the raw pointer, and putting the raw pointers in vectors. What is the right way to do it?

+2  A: 

In my opinion, in C++, you should always have to justify the use of an unguarded pointer.

There could be many valid reasons: a need for very high performance, for very low memory usage, for dealing with legacy libraries, because of some issue with the underlying data structure the pointer is storing. But [dynamically allocated] pointers are somewhat 'evil', in that you have to deallocate the memory at every possible execution path and you will almost certainly forget one.

Oliver N.
The goto statement may be used (in non-spagetti mode) to ensure that every exit point goes through a block of code... ergo: you can roll your own throw-try-catch-finally... we did... it works brilliantly, and without the massive overheads of Java/C#'s exception handling.
corlettk
A: 
A: 

I wouldn't put raw pointers in vectors.

In case they use auto_ptr or boost::scoped_ptr, they can't use (or return) anything but raw pointers. That could explain their way of coding, i guess.

Benoît
I don't understand what you mean by "In case they use auto_ptr or boost::scoped_ptr, they can't use (or return) anything but raw pointers." Do you mean that a function can't return an auto_ptr, or simply that sharing can't be expressed with auto_ptr?
Éric Malenfant
I simply mean that auto_ptr (or scoped_ptr) can't be used in a container.
Benoît
+11  A: 

There is no "right" way. It really depends on the context.

You can internally handle memory with a smart pointer and externally give references or raw pointers. After all, the user of your interface doesn't need to know how you manage memory internally. In a synchronous context this is safe and efficient. In an asynchronous context, there are many pitfalls.

If you're unsure about what to do you can safely return smart pointers to your caller. The object will be deallocated when the references count reaches zero. Just make sure that you don't have a class that keeps smart pointers of objects for ever thus preventing the deallocation when needed.

As a last note, in C++ don't overuse dynamically allocated objects. There are many cases where you don't need a pointer and can work on references and const references. That's safer and reduces the pressure on the memory allocator.

Edouard A.
+4  A: 

It depends on what the meaning of the pointer is.

When returning a shared_pointer, you are syntactically saying "You will share ownership of this object", such that, if the the original container object dies before you release your pointer, that object will still exist.

Returning a raw pointer says: "You know about this object, but don't own it". It's a way of passing control, but not keeping the lifetime tied to the original owner.

(in some older c-programs, it means "It's now your problem to delete me", but I'd heavily recommend avoiding this one)

Typically, defaulting to shared saves me a lot of hassle, but it depends on your design.

Todd Gardner
+4  A: 

I would never return a raw pointer, instead I would return a weak_ptr to tell the user of the pointer that he doesn't have the control over the resource.

If you return a weak_ptr its very unlikely that there will be dangling pointers in the application.

If there is a performance problem I would return a reference to the object and a hasValidXObject method.

TimW
+1  A: 

I typically return "owning"/"unique" smart pointers from factories or similar to make it clear who is responsible for cleaning up.

Johann Gerell
Eh? Why downvoting that without explanation? Someone with too little large-scale enterprise app code under his belt?
Johann Gerell
+1  A: 

I follow the following guidelines for passing pointers arguments to functions and returning pointers:

boost::shared_ptr

API and client are sharing ownership of this object. However you have to be careful to avoid circular references with shared_ptr, if the objects represent some kind of graph. I try to limit my use of shared_ptr for this reason.

boost::week_ptr / raw pointer

API owns this object, you are allowed share it while it is valid. If there is a chance the client will live longer than the api I use a week_ptr.

std::auto_ptr

API is creating an object but the client owns the object. This ensures that the returning code is exception safe, and clearly states that ownership is being transferred.

boost::scoped_ptr

For pointers to objects stored on the stack or as class member variables. I try to use scoped_ptr first.

Like all guidelines there will be times when the rules conflict or have to be bent, then I try to use intelligence.

iain
I prefer boost::unique_ptr instead of std::auto_ptr because it's harder to move ownership by accident.
romkyns
Thanks I hadn't spotted unique_ptr, as it is not in the boost book. It looks good.
iain
A: 

const boost::shared_ptr &getInternal() {return m_internal;}

This avoids a copy.

Sometimes you'll like to return a reference, for example:

  • Y &operator*() { return *m_internal; }
  • const Y &operator*() const { return *m_internal; }

This is good too only if the reference will be used and discarded inmediately. The same goes for a raw pointer. Returning a weak_ptr is also an option.

The 4 are good depending on the goals. This question requires a more extensive discussion.