best practice when returning smart pointers

Question

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?

Answer 1

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.

Answer 2

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.

Answer 3

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

Answer 4

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

This example https://ideone.com/qJnzva shows how to return a std::unique_ptr that will be deleted when the scope of the variable that the caller assigns the value to goes out of scope.

While it's true that the smart pointer deletes its own pointer, the lifetime of the variable holding the smart pointer is 100% controlled by the caller, so the caller decides when the pointer is deleted. However, since it's a "unique" and "owning" smart pointer, no other client can control the lifetime.

Answer 5

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.

Answer 6

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.

Answer 7

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.

Answer 8

depends on your goals.

blindly returning smart ptr to internal data might not be a good idea (which is very sensitive to the task you're trying to solve) - you might be better off just offering some doX() and doY() that use the pointer internally instead.

on the other hand, if returning the smart ptr, you should also consider that you'll create no mutual circular references when objects end up unable to destroy each other (weak_ptr might be a better option in that case).

otherwise, like already mentioned above, performance/legacy code/lifetime considerations should all be taken into account.

Answer 9

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.