🎉 Celebrating 25 Years of GameDev.net! 🎉

Not many can claim 25 years on the Internet! Join us in celebrating this milestone. Learn more about our history, and thank you for being a part of our community!

Best practices and std::unique_ptr

Started by
7 comments, last by Juliean 4 years, 1 month ago

Trying to use best practices for C++ and I see that you should be using std::unique_ptr now. And coming from using raw pointers only I have some questions.

When it comes to returning functions, if I return a std::unique_ptr like:

//Declared as member variable of MyObject class
std::unique_ptr<InternalObject> internalObject;

//Return function of MyObject class to get internalObject as a std::unique_ptr
std::unique_ptr<InternalObject> getInternalObject() {
	return internalObject;
}

What am I really returning here? I read that this more or less “gives up” ownership of the pointer and does std::move. If this is true what happens to my internalObject variable?

Assuming the above is true still. If I don't want to transfer ownership I assume I do this instead

//Declared as member variable of MyObject class
std::unique_ptr<InternalObject> internalObject;

//Return function of MyObject class to get internalObject as pointer
InternalObject* getInternalObject() {
	return internalObject.get();
}

This kind of seems wrong because I though the whole point of smart pointers like std::unique_ptr was so you don't touch/handle the raw pointer. Because couldn't I just do something like

//Declared as member variable of MyObject class
std::unique_ptr<InternalObject> internalObject;

//Return function of MyObject class to get internalObject as pointer
InternalObject* getInternalObject() {
	return internalObject.get();
}

//Somewhere in my program
MyObject myObject;
InternalObject* value = myObject.getInternalObject();
delete value;

What is the proper way I should be using / returning std::unique_ptr?

Also I noticed that you cannot do things like

//Internal Object
class InternalObject
{
	private:
		InternalObject();
		~InternalObject();
}

//Inside MyObject class
std::unique_ptr<InternalObject> createInternalObject() {
	//Compiler Error: Mad about private destructor?
	return std::unique_ptr<InternalObject>(new InternalObject()); 
}

So how should you be working with std::unique_ptr when I want to declared constructor/destructors as private?

Advertisement

noodleBowl said:
What am I really returning here? I read that this more or less “gives up” ownership of the pointer and does std::move. If this is true what happens to my internalObject variable?

Nothing happens to the internalObject-variable at all. Each object can only be held by one unique_ptr, and if you return a unique_ptr you can only do so by move. The thing that you moves essentially will hold the same object when you assign the return-value of the function to another variable. If you drop the call w/o anything to store the object into, it gets deleted.

noodleBowl said:
This kind of seems wrong because I though the whole point of smart pointers like std::unique_ptr was so you don't touch/handle the raw pointer. Because couldn't I just do something like

Yes, sure, but you could also do:

std::unique_ptr<InternalObject> internalObject;

delete internalObject.get();

What is the proper way I should be using / returning std::unique_ptr?

The first rule is this: You never delete an object yourself. Period.

The second rule then is: Return and pass raw pointer (or better, reference if the value can never be null), if the lifetime of the receiving function is guranteed to outlife the stored object. This simply means, whenever you have a function that returns immediately and doesn't hold on to the object for long (or if whatever is holding on to the object has a shorter lifetime as whever the objects lifetime is stored).

Then, in the simplest case you have one place in the program where your objects lifetime is controlled. That is where you use your std::unique_ptr. Nothing else has or should know that its backed by a unique_ptr. As above, you hand out references and pointers. You only pass std::unique_ptr around to transfer the ownership of the created objects.

For more complex cases, you might then turn to std::shared_ptr. I personally only use shared ownership sparcely, for situations where its difficult to control the lifetime between different systems; for holding objects in a script-layer or for editor-specific code.

So how should you be working with std::unique_ptr when I want to declared constructor/destructors as private?

std::unique_ptr can take a custom deleter (see the second template argument), and I think you could pass a static function/lambda as that. But I haven't dealt with that myself, I rarely declare destructors private. I also think you are kind of lying if you got std::unique_ptr to work that way. The owner of the unique_ptr gets to decide where and when to delete the object, but by making the destructor private you tell the user that you do not indend for them to freely destroy the object.

Oh, one more thing. Prefer to create your unique_ptrs with make_unique:

return std::make_unique<InternalObject>();

Less code, more descriptive and less prone to errors/leaks.

noodleBowl said:

//Declared as member variable of MyObject class
std::unique_ptr<InternalObject> internalObject;

//Return function of MyObject class to get internalObject as a std::unique_ptr
std::unique_ptr<InternalObject> getInternalObject() {
	return internalObject;
}

What am I really returning here? I read that this more or less “gives up” ownership of the pointer and does std::move. If this is true what happens to my internalObject variable?

I'm not sure whether this implicitly does a move or not (I would expect to explicitly std::move), but I think unique_ptr might not be what you're looking for. There's a few ways to handle ownership of a particular member:

  1. I will always be the only consumer of this object.
    Regular member variable
  2. I will always own this object, but others need to access it directly.
    Regular member variable, return a reference to it.
  3. I own this object for now, but will want to transfer ownership at some point.
    Unique ptr, and make sure nothing will break once it's transferred.
  4. I share ownership of this object, and it should exist while any owner needs it.
    Shared ptr

If you're needing to write a getter for that member, you probably need something that isn't a unique ptr (unless you're using a very particular scheme and every consumer is aware of it, but this would likely be fragile).

noodleBowl said:

//Internal Object
class InternalObject
{
	private:
		InternalObject();
		~InternalObject();
}

//Inside MyObject class
std::unique_ptr<InternalObject> createInternalObject() {
	//Compiler Error: Mad about private destructor?
	return std::unique_ptr<InternalObject>(new InternalObject()); 
}

So how should you be working with std::unique_ptr when I want to declared constructor/destructors as private?

Unique and shared ptrs need to be able to call the destructor when they're no longer referenced. You might be able to get around it using friend classes, but you probably shouldn't do that. The actual solution is to have a public destructor, which shouldn't be an issue because there won't be a valid case for someone calling it manually (so it's easy to catch the bug if someone does do it).

If you really care, you can pass a custom deleter to the smart pointer, but it doesn't seem necessary to me. https://stackoverflow.com/questions/8202530/how-can-i-call-a-private-destructor-from-a-shared-ptr

It seems to me that unique pointers are intended for passing objects around between different owners such that at all times, there is only one owner with access and deleting the object is handled properly.

So far, I have not run into such a need and thus have not yet used unique pointers.

Your first guess is correct, if you return your std::unique_ptr to a caller, you lose the object unti you query it again or until it is given to you again. Your unique pointer becomes empty. The raw pointer access looks more like an escape hatch to give raw access if you need it. I don't think you should be handing out raw pointers to it, as it defeats the idea of one owner with access.

On the other hand however, this is C++, where the programmer knows what he/she is doing, so if you decide otherwise, C++ is fine with it. This is also how you should see deleting the raw pointer. Yes you can do it, but you know what you're doing. Raw pointers aren't that bad I think, as deleting an object containing a raw pointer doesn't delete the pointed-at object, but perhaps you want a shared pointer instead here??

Obviously, you can always write a non-owning wrapper class around a pointer, but not sure how that is more useful than the raw pointer itself, especially if you eventually still have to add an escape hatch and give access to the raw pointer any way.

Your ‘createInternalObject’ doesn't have ‘private’ access so it fails. The intention is that nobody outside the class has access which is useful at times (it gives control who can make a new instance). Making it a ‘friend’ function likely works, another option should be making it a static member function. For more details, see https://en.cppreference.com/w/cpp/language/access

Alberth said:
It seems to me that unique pointers are intended for passing objects around between different owners such that at all times, there is only one owner with access and deleting the object is handled properly.

No, the part about “only one owner with access” is an incorrect assumption. std::unique_ptr is for unique-ownership/lifetime-management. That does not mean that nobody else gets access to the object. You might still hand the object owned by a unique-ptr to a function that does some calculations and returns a result, or let users of the class access the owned object as a reference.

Ownership and access are two very different aspects in C++, and if you correctly use std::unique_ptr for everytime when something really takes lifetime of an object, and pointer/reference for when something simply uses the object without assuming lifetime, you get an interface that is much easier to understand then if a function just magically assumes that it can take a object*, own it and delete it at some point.

So by that logic, std::unique_ptr really comes as a clean replacement for all the places in your code where you would normally “new” an object, store it somewhere and later delete it. Thus you might want to rethink about not using it at all ?

@Juliean

Juliean said:
Nothing happens to the internalObject-variable at all. Each object can only be held by one unique_ptr, and if you return a unique_ptr you can only do so by move. The thing that you moves essentially will hold the same object when you assign the return-value of the function to another variable. If you drop the call w/o anything to store the object into, it gets deleted.

So something does happen right? My original std::unique_ptr becomes “empty” since it gets moved

//Declared as member variable of MyObject class
std::unique_ptr<InternalObject> internalObject;

//Return function of MyObject class to get internalObject as pointer
std::unique_ptr<InternalObject> getInternalObject() {
	return internalObject.get();
}

//Some function that moves the pointer to a new owner
void DoWork() {
	//Move myObject's internalObject tp newOwner
	//newOwner now owns internalObject. internalObject's InternalObject pointer is now empty
	std::unique_ptr<InternalObject> newOwner = myObject.getInternalObject();
}

//Some function that drops the pointer
void DoWork() {
	//Move myObject's internalObject to a "new owner"
	//But since there is no assignment it to anyone it is dropped
	//internalObject's InternalObject pointer is now empty / deleted
	myObject.getInternalObject();
}

@archduke

Archduke said:
Unique and shared ptrs need to be able to call the destructor when they're no longer referenced. You might be able to get around it using friend classes, but you probably shouldn't do that. The actual solution is to have a public destructor, which shouldn't be an issue because there won't be a valid case for someone calling it manually (so it's easy to catch the bug if someone does do it).

I feel like if a mark a constructor as private then the destructor should also be private on the grounds that if I don't want anyone creating this object then I must not want anyone being able to delete this object. Maybe I had this wrong the entire time.

I make private constructors when I don't want someone/something to being able to directly “new up” the object and I intend those objects to come from somewhere else. Eg: objects that must be made via a factory or a singleton where the "owner" of the singleton is allowed to create and delete only via friend declaration

@Alberth

I'm actually a big fan of just using the raw pointer's. I really didn't feel like they were wrong or bad. So this:

Alberth said:
On the other hand however, this is C++, where the programmer knows what he/she is doing

is a very true statement for me. If I mess up then I mess up. Its my fault. But unfortunately this isn't the general sediment and raw pointers will eventually be phased out

noodleBowl said:
But unfortunately this isn't the general sediment and raw pointers will eventually be phased out

I don't think that will happen, it is a core concept in computer systems and it will break all existing code, not to mention compatibility with C.

However, a raw pointer brings a few risks that you can easily avoid by using references or the memory management objects. That's how I see these new conventions, standard solutions for standard problems that just work without thinking about it. It gives us more time to think about the non-standard bits which are imho much more interesting to spend time on.

Juliean said:
No, the part about “only one owner with access” is an incorrect assumption.

Good point, I clearly haven't coded anything complicated enough in C++ for too long. Thanks.

noodleBowl said:
So something does happen right? My original std::unique_ptr becomes “empty” since it gets moved

Ah, now I get what you are doing.

So then the answer is: neigther your original version nor what you posted now will compile. A unique_ptr cannot be copied or implicitely created from a pointer w/o explicitely specifying the constructor. So the correct version, which yes will reset “internalObject” to hold a nullptr, is:

std::unique_ptr<InternalObject> getInternalObject() {
	return std::move(internalObject);
}

Thats the good thing about unique_ptr, you are never accidentially moving the ownership unless you move (or if you have a local variable; which I originally thought you have, then the move will happen automatically).

This topic is closed to new replies.

Advertisement