Overusing smart pointers?

Started by
31 comments, last by sundersoft 11 years, 10 months ago
For factory functions I prefer to return a auto_ptr or unique_ptr (depending on the compiler support I have to work with) as then that says that this is a new object that no one else has any references to rather than being a potentially shared object. Since shared_ptr has a constructor that takes an auto_ptr or unique_ptr, you can sink it into a shared_ptr if you want to do sharing, but the factory doesn't force sharing overhead on you if you don't want it.
Advertisement
I won't too far into the debate over shared pointers except to say that I am a big fan, they are extremely useful in the right circumstances, but as with most programming constructs they can, of course, be overused. So take the time to learn as much as you can about them if you're going to use them.

I will also say that I've found unique_ptr is very handy for creating temporary buffers of dynamic size, or ones of static size that you don't necessarily want eating up your stack.

i.e:

std::unique_ptr<char[]> buffer( new char[64 * 1024] );

Then you don't need to worry about exception safety or calling delete in every exit point of your function.

3) unique_ptr .. which is like 2 but if you are willing to write unique_ptr<blabla>, .get() and all that nonsense over and over again to AVOID 1 SINGLE CALL TO "delete" in your destructor.


Somewhat agree, somewhat disagree with this point.

I don't use exceptions in my C++, but using a unique or scoped pointer makes your code more exception safe.

Even if you don't use exceptions, for allocations within a function, it take out a lot of the programmer overhead of memory management. Then, for consistency's sake, I use scoped pointer wrapper for member variables too.

I do agree that littering code with myPtr.get() is a bit of a pain in the neck though!
Even if you are not explicitly using exceptions yourself the standard library still does and new will also throw unless you explicitly use "nothrow"... But then if you are not using exceptions you will probably also not catch those and the program will simply terminate.
Assuming the pointer must not be null, is there any reason to use pointer.get() over &*pointer? I've been using the latter as it works for pretty much any pointer-like type (raw pointers, boost smart pointers, qt smart pointers).

Assuming the pointer must not be null, is there any reason to use pointer.get() over &*pointer? I've been using the latter as it works for pretty much any pointer-like type (raw pointers, boost smart pointers, qt smart pointers).


From a functional standpoint, no. From a code clarity standpoint, yes. pointer.get() is clear in it's intent, &*pointer looks like a bug.

That being said, if your code is littered with .get() then you may have a more fundamental code smell; overusing pointers, period. You should almost always prefer to pass by const reference rather than by pointer unless you have a good reason to pass a pointer (i.e. an output parameter, a raw buffer, or interfacing to a third-party library). So most access to the object should either be pointer-> or *pointer rather than pointer.get().
The only issue with over-using smart pointers is that they will make you feel safe while coding.
That feeling of safety might lead to a messy flow in your game. Using normal pointers forces you to have a clean flow in your code, a flow of initialization, the ownership for each object is clearly defined, etc.
I've seen this a lot in school projects (university), and to some degree even in some engines used to make games :).
You can end up with some object shared between two or three others, with no clearly defined owner and this might give you a lot of headaches.
I have some policies that I always stick with in my personal C++ development.

1, Always use clear and explicit ownership.
2, Prefer scoped pointer (unique_ptr) to raw pointer.
3, Avoid shared pointer unless there is good reason. Policy 1 implicits that shared pointer should be avoided. The only good reason to use shared pointer I've found recently is that, when there is no clear ownership of the resource. Such as in a script binding, the C++ resource may be freed by script engine without knowing by your code.

With those policies, I use scoped pointer extensively but I never overuse shared pointer, and I rarely get memory leak or bad pointer.

Just my 2 cents

https://www.kbasm.com -- My personal website

https://github.com/wqking/eventpp  eventpp -- C++ library for event dispatcher and callback list

https://github.com/cpgf/cpgf  cpgf library -- free C++ open source library for reflection, serialization, script binding, callbacks, and meta data for OpenGL Box2D, SFML and Irrlicht.

I've started changing alot of my code now.

But I have a case where I use a unique_ptr as its a factory function that creates the object.. I then move this pointer into a owning container.. but I also have to set a currently selected object in another class.. should I rather use a shared_ptr in this case and use a weak_ptr for where I set the currently selected object? Or should I use a raw pointer for the selected object and get the raw pointer out of the unique pointer like I do below?


class A
{

};

class B
{
public:
void SetCurrent(A* current) { m_current = current; }
private:
A* m_current;
};

B b;
std::vector<std::unique<A>> vec;
std::unique<A> p0 = CreateA();
std::unique<A> p1 = CreateA();
vec.push_back(std::move(p0));
vec.push_back(std::move(p1))
b.SetCurrent(p1.get());

should I rather use a shared_ptr in this case and use a weak_ptr for where I set the currently selected object? Or should I use a raw pointer for the selected object and get the raw pointer out of the unique pointer like I do below?


Don't mix raw pointers with smart pointers, by storing raw pointers you completely negate the safety and validity of your smart pointers. You can .get() the raw pointer for local operations where there is no risk of memory corruption.

In this case you probably want to pass a shared_ptr to A and store it as either a shared_ptr or weak_ptr. If you actually intend to transfer ownership of A into B, then you can pass unique_ptr.

unique_ptr is not as useful when you have a pointer that you need to pass around a lot. unique_ptr is very useful for heap-allocated objects within initialization functions that might fail early, relieving you of a lot of ugly cleanup work. I would also agree that unique_ptr is useful for returning objects from a factory, but not so much if there is a cache involved. If there is a cache, what you actually have is shared ownership (this cache holds onto one reference while another reference is grabbed by the object calling the factory).

This topic is closed to new replies.

Advertisement