32 replies to this topic

[Nanook]

I have a feeling that I might be overusing smart pointers.. Does anyone know of a good article on when to use them and when not to?

[Rene Z]

Probably not.

I always use smart pointers, unless there's a good reason not to. A good reason can be performance in tight loops or something like that, but those cases are rare and should never leak out of your interfaces. Note that using smart pointers doesn't mean 'don't think about ownership', which often results in overusing shared pointers. Quite the opposite in fact: different types of smart pointers are a great way to explicitly state ownership of an object.

[kunos]

Actually Stoustrup himself suggests to use smart pointers as "last resort", expecially shared_ptr . The danger of introducing smart pointers as standard in C++ is, IMO, the urge that people have to use them everywhere, and I believe that is wrong for a series of reasons but the top one is: they really make C++ uglier than already is, having this const shared_ptr<Node>& all over the place is really ugly.

C++ object lifetime should be managed with the following priority:

1) Stack based. This is the way C++ was designed for. If you can make it stack based, do it. Cleanest and fastest code is obtained this way.
2) Clean ownership heap based with new and delete. One guy creates the object, same guy is responsible to delete it.
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.
4) shared_ptr . That is, when the ownership is _really_ SHARED, so you can't possibly predict the lifetime and ownership of an object and this is used and kept alive by different objects.

So while I can't possibly think about using option 3 ever.. I know there are scenarios where 4 is useful. I use shared_ptr for materials for example, and it works ok.
Also, shared_ptr can create leaks that are REALLY hard to detect if you manage to create a reference loop.

All the above is IMO IMO IMO .. smart pointes just made it into the C++11 standard and there is still no book about best practices out yet... so the jury is still out on this one.

Edited by kunos, 17 June 2012 - 11:04 AM.

[e‍dd]

Actually Stoustrup himself suggests to use smart pointers as "last resort", expecially shared_ptr . The danger of introducing smart pointers as standard in C++ is, IMO, the urge that people have to use them everywhere, and I believe that is wrong for a series of reasons but the top one is: they really make C++ uglier than already is, having this const shared_ptr<Node>& all over the place is really ugly.

IMHO, correctness should come before aesthetics every time. typedefs can be a big help, as can auto if available.

C++ object lifetime should be managed with the following priority:

1) Stack based. This is the way C++ was designed for. If you can make it stack based, do it. Cleanest and fastest code is obtained this way.

I definitely agree with this.

2) Clean ownership heap based with new and delete. One guy creates the object, same guy is responsible to delete it.
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.

If you've got unique_ptr, you've quite possibly got auto too. You get brevity with additional safety for free. And the more you standardise on smart pointers, the less you need .get().

Besides, without the use of something like unique_ptr, one would typically be required to make more than one call to delete where error handling occurs, early returns, inside surrounding catch clauses, etc.

[Matias Goldberg]

I see a flamewar coming, let's keep it calm.

The problem with auto is that it can also be overused. You get safety and smaller lines of code. But it's also harder to grok for another human being who didn't write the code. You have no idea what the variable type holds unless you make more use of advanced IDE tools. Furthermore I'm starting to see programmers using auto for almost every single variable they declare (being completely unnecessary btw).
Using auto inside a template is incredibly useful because by definition, templates don't know the type of objects that are being passed into them until compiled.
But IMHO, outside of a template, the usage of auto should only be used as a last resort; typically where using auto has a real benefit over explicitly declaring the type of the variable. This isn't an old problem, interpreted languages (Python, Lua) already have this issue. It's not a problem about safety, style, or aesthetics. It's a problem of deeply understanding what your (or someone else's) code is doing, which is very important in close-to-the-metal languages like Asm, C & C++.

In Python & Lua, this problem is alleviated because one can write "print( variable )" and they also have an interactive interpreter. In C++ we have none of that.

[njpaul]

I tend to rarely use smart pointers. When I first was introduced to them, I overused shared_ptr, and I would often end up with some interesting crashes in destructors on program exit. Then I started thinking more about why I actually needed them for my situation, and it turns out I didn't. I often have manager classes that are the sole place for creation and destruction of whatever resource they're managing. When I need a pointer to a resource, I ask the manager for it.

Not that there aren't any reasons to use smart pointers, but I tend to structure code in a way where I don't need them. Sometimes this can't be done, and sometimes it'd just be too much work to refactor the code. I know of a multi-million line program that is littered with shared pointers, and occasionally those developers will run into a problem with them.

[japro]

shared_ptr can be overused unique_ptr on the other hand has no overhead anyway (except maybe for the syntactical one). But the benefits you get are a way lower chance to produce memory leaks, exception safety etc... Also the "overhead" of writing unique_ptr is often compensated by the fact that you don't even need an explicit constructor on lots of cases.

Anyone who dismisses smart pointers with "all it saves you is a delete in the destructor" has apparently never heard of exception safety.

[wack]

So far I have mostly used unique_ptr (or the old auto_ptr), because it's a simple way of getting exception safety when allocating something locally.

Can't think of any time when I've had to resort to shared_ptr, there always seems to be a better option.

[Nanook]

Aah great.. lots of good stuff here.. I think actualy my greatest problem is that I'm using the heap more than I should..

[Álvaro]

In my own code I tend to use local variables and standard containers the vast majority of the time, which makes explicit use of pointers rather infrequent. I think this style of programming has saved me from many headaches, and I strongly advocate it.

When I do have to use a pointer (e.g., for the return value of a factory), I often end up using a shared_ptr because I don't trust the caller of that code (usually me) with not messing up the ownership. But I don't feel very strongly about this choice and perhaps I'll change it in the future.

[SiCrane]

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.

[krippy2k8]

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.

[Neilo]

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!

[japro]

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.

[Slavik81]

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).

[krippy2k8]

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().

[clickalot]

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.

Edited by clickalot, 19 June 2012 - 02:01 AM.

[wqking]

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

[Nanook]

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());

Edited by Nanook, 19 June 2012 - 07:10 AM.

[y2kiah]

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).