Passing unique_ptr to functions

Question

I'm trying to "modernize" some existing code.

• I have a class which currently has a member variable "Device* device_".
• It uses new to create an instance in some initialization code and has a "delete device_" in the destructory.
• Member functions of this class call many other functions that take a Device* as a parameter.

This works well, but to "modernize" my code I thought I ought to change the variable to be defined as "std::unique_ptr<Device> device_" and remove the explicit call to delete, which makes the code safer and generally better.

My question is this -

• How should I then pass the device_ variable to all of the functions that need it as a paramater?

I can call .get to get the raw pointer in each function call. But that seems ugly and wastes some of the reasons to use a unique_ptr in the first place.

Or I can change every function so that instead of taking a parameter of type "Device*" it now takes a paramater of type "std::unique_ptr& ". Which (to me) somewhat obfuscates the function prototypes, and makes them hard to read.

What is best practice for this? Have I missed any other options?

Solution 1

In Modern C++ style, there are two keys concepts:

• Ownership
• Nullity

Ownership is about the owner of some object/resource (in this case, an instance of Device). The various std::unique_ptr, boost::scoped_ptr or std::shared_ptr are about ownership.

Nullity is much more simple however: it just expresses whether or not a given object might be null, and does not care about anything else, and certainly not about ownership!

---

You were right to move the implementation of your class toward unique_ptr (in general), though you may want a smart pointer with deep copy semantics if your goal is to implement a PIMPL.

This clearly conveys that your class is the sole responsible for this piece of memory and neatly deals with all the various ways memory could have leaked otherwise.

---

On the other hand, most users of the resources could not care less about its ownership.

As long as a function does not keep a reference to an object (store it in a map or something), then all that matters is that the lifetime of the object exceeds the duration of the function call.

Thus, choosing how to pass the parameter depends on its possible Nullity:

• Never null? Pass a reference
• Possibly null? Pass a pointer, a simple bare pointer or a pointer-like class (with a trap on null for example)

---

Solution 2

It really depends. If a function must take ownership of the unique_ptr, then it's signature should take a unique_ptr<Device> bv value and the caller should std::move the pointer. If ownership is not an issue, then I would keep the raw pointer signature and pass the pointer unique_ptr using get(). This isn't ugly if the function in question does not take over ownership.

Solution 3

I would use std::unique_ptr const&. Using a non const reference will give the called function the possibility to reset your pointer.
I think this is a nice way to express that your called function can use the pointer but nothing else.
So for me this will make the interface easier to read. I know that I don't have to fiddle around with pointer passed to me.

Solution 4

The best practice is probably not to use std::unique_ptr in this case, although it depends. (You generally should not have more than one raw pointer to a dynamically allocated object in a class. Although this also depends.) The one thing you don't want to be doing in this case is passing around std::unique_ptr (and as you've noticed, std::unique_ptr<> const& is a bit unwieldy and obfuscating). If this is the only dynamically allocated pointer in the object, I'd just stick with the raw pointer, and the delete in the destructor. If there are several such pointers, I'd consider relegating each of them to a separate base class (where they can still be raw pointers).

Solution 5

That may be not feasible for you but a replacing every occurence of Device* by const unique_ptr<Device>& is a good start.

You obviously can't copy unique_ptrs and you don't want to move it. Replacing by a reference to unique_ptr allows the body of the existing functions' bodies to keep on working.

Now there's a trap, you must pass by const & to prevent callees from doing unique_ptr.reset() or unique_ptr().release(). Note that this still passes a modifiable pointer to device. With this solution you have no easy way to pass a pointer or reference to a const Device.

View more solutions

Comments

[James Kanze]

Why would this change make the code safer and generally better? From your description, I would say that just the opposite is true.

[juanchopanza]

@James Kanze I agree, unique_ptr doesn't seem to buy anything in this case.

[jcoder]

You may well be right. I like the "safety" of unique_ptr in that you don't have to remember to delete it, but it does seem to have some cost in this case.

[James Kanze]

The real safety of unique_ptr is that you don't need to surround the pointer with a try block to ensure that it is deleted in case of an exception. Which means that it doesn't buy you much in an object with a destructor, unless you have particular constraints in the constructor. (Whether you put the delete in the destructor, or use unique_ptr, comes out to about the same thing in terms of what you have to remember to do.)

[jcoder]

@James Kanze yes that makes sense. Interesting

[David Rodríguez - dribeas]

@JamesKanze if the constructor does not complete (throws an exception), the use of std::unique_ptr will avoid leaking memory in the simplest way. It is not the only way, but it is simple and future proof (I.e. later modifications to the constructor won't risk leaking). In the worst case it should be no worse than the handcrafted approach. Why do you say that in this case the opposite [might be] true?

[James Kanze]

@DavidRodríguez-dribeas There is no simple, one size fits all answer. From his description, I see no problem which std::unique_ptr would help solve. It's just added complexity and obfuscation. In more complicated situations, it depends. Most of the time, a cleaner solution would be to delegate to a base class which is responsible for just one thing, this object. (A very good example of this is the g++ implementation of std::vector.) In other cases, it might make sense to have a unique_ptr member, but in my experience, they aren't very common.

[jcoder]

So to be clear, adding const there will mean I can't change the unique_ptr in any way, but I can call non const functions of the contained object via the unique_ptr?

[jcoder]

Yes I don't want to take ownweship, just use the pointed to object in the function.

[mkaes]

@JohnB: Yes the const prevents the called function to call e.g reset on your unique_ptr. But still as long as the pointer inside is not const you can call non const functions.

[zabulus]

Yes you can. get() const method of the unique_ptr returns non-const pointer to the object guarded by unique_ptr

[juanchopanza]

@mkaes I owuld argue that passing a const reference to a unique pointer is less clear than passing a (possibly cv qualified) raw pointer in this case.

[mkaes]

@juanchopanza: I would argue that with a raw pointer it is always difficult to make ownership clear. A unique_ptr solves this in a nice way.

[jcoder]

Hmm, references... I could make my functions take a "Device& device" parameters and then in the calling code use *device_ as that will still work with unique_ptr and I know device_ can't be null (and can test that with asserts). Now I need to ponder that for a bit and decide if it's ugly or elegant!

[James Kanze]

@mkaes If the design doesn't make it clear, then using unique_ptr won't change anything. If you understand the role and the responsibilities of the class, then it should be clear whether you need a delete in the destructor or not. If you don't know what the class is supposed to do, then unique_ptr won't tell you.

[Matthieu M.]

@JohnB: well, my biaised point of view is that eradicating the possibility of nullity at compile-time is much safer :)

[David Rodríguez - dribeas]

@JohnB: Completely agree with Matthieu here, and I would even go further, why do you want to dynamically allocate an object whose lifetime is bound by the lifetime of the object that holds the pointer? Why not declare it as a plain member with automatic storage?

[Marc van Leeuwen]

Maybe I'm mistaken, but it would seem to me that taking a parameter of type const std::unique_ptr<T>& is always sub-optimal. With the const you are saying, I won't take the object (ownership) away from you (the caller). But with the std::unique_ptr part you are saying you must be the (direct) owner of the object pointed to. But given that I (called function) am not going to assume ownership, I have no business in knowing who owns. Note that if caller "rents" a pointer (has access but not ownership) then it cannot build a unique_ptr from it without creating double ownership.

[Goswin von Brederlow]

Since const unique_ptr<Device>& is unwieldy I would typedef that to Device_t or similar.

[Admin]

You can also pass a const reference to the unique_ptr if ownership isn't an issue. Some people use pointer-reference dichotomy to simulate Pascals's in-out formal parameter semantics (Google C++ style guide).