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?
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 aroundstd::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 thedelete
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).That may be not feasible for you but a replacing every occurence of
Device*
byconst unique_ptr<Device>&
is a good start.You obviously can't copy
unique_ptr
s and you don't want to move it. Replacing by a reference tounique_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 doingunique_ptr.reset()
orunique_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 aconst Device
.In Modern C++ style, there are two keys concepts:
Ownership is about the owner of some object/resource (in this case, an instance of
Device
). The variousstd::unique_ptr
,boost::scoped_ptr
orstd::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:
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.
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 shouldstd::move
the pointer. If ownership is not an issue, then I would keep the raw pointer signature and pass the pointer unique_ptr usingget()
. This isn't ugly if the function in question does not take over ownership.