6

Regarding pointers which are members of classes. Should they be of a smart pointer type or is it enough to simply deal with them in the destructor of the class they are contained in?

That Realtor Programmer Guy
  • 826
  • 6
  • 18

2 Answers2

14

No, it's not enough to deal with the raw pointer in the destructor. You also have to deal with it in the copy constructor and assignment operator functions. It's usually not enough to simply copy the pointer (which the compiler will happily do for you); you probably really want to make a copy of the resource it points to. And you have to remember to gracefully handle self-assignment without leaking the resource. Maybe you don't want to support copying at all, so you have to remember to explicitly disable those functions. Maybe you want to give out handles to that resource, so you have to keep track of when it's safe to be freed. What if an exception is thrown in the middle of your constructor? What if you aren't the one throwing it? Maybe there's another edge case I haven't thought of.

(Note: I've debugged more than enough of this kind of code that gets it wrong, hence my somewhat harsh tone.)

If you want to get it right on the first try, then yes, you should go with a smart pointer type such as boost::scoped_ptr or std::tr1::shared_ptr.

Michael Kristofik
  • 1,503
  • 9
  • 15
  • Personally I use deep_copy pointers (unfortunately not part of boost) since the semantics is pretty much what I normally want from a pointer (if I don't want copy, I'll use scoped_ptr). shared_ptr lack ownership and, as such, I use it only in extreme case where ownership is hard (or impossible) to determine. – n1ckp Mar 23 '11 at 14:46
  • add here point about exceptions thrown from middle of constructor :) – maxim1000 Mar 23 '11 at 20:29
  • 1
    @user20933, the whole point here is to get used to RAII. I will say there's a certain satisfaction in writing a resource-managing class (where you have to do all this stuff yourself) and *knowing* you got it right. Elegant designs are easy for others to use, even if the innards get a little messy. – Michael Kristofik Mar 24 '11 at 10:56
2

Well if they are only members, I prefer not bother with delete and use boost::scoped_ptr. For me that's the simpler way to manage it.

Now, if you don't have access to boost, or don't want to use it, you can make you own simple smart pointer that simply does the delete for you in it's destructor. It's really easy and I let you do it as an exercise, if you need it.

In the end, only if I'm "forced" to I'll use delete in the destructor of the owner class. The problem is that I know it will change in the future and if we wait enough, it will change in a way that will make the delete wrong. When that happen, nobody will remember there was a delete, until it crashes...

So, it's better to setup the destruction rule at the creation or definition point instead of somewhere else in the code base (somewhere nobody will remember because it's far from the definition point).

That's why we use smart pointers where we can.

As always, it's not a good idea to use smart pointers for everything, but in this specific case, if your member is only created at owner construction and destroyed at owner destruction, make your life easy, use a smart pointer.

Klaim
  • 14,832
  • 3
  • 49
  • 62
  • *"When that happen, nobody will remember there was a delete, until it crashes"* can you help me understand with an example? That sounds true enough, I've just drawn a blank. Also, when might you be forced to? – That Realtor Programmer Guy Mar 24 '11 at 05:40
  • Forced to do what? I'm not sure what kind of example you need... the idea is simply that when you look at the definition of the variable, you see the type and allocation store and you understand what it's whole behaviour is, including lifetime. If you're using new, then you know that there is a hole in your comprehension, until you see where the corresponding delete is. If there is more than one, it's hell, because there is no clear "rule" to the delete call. If you use a smart pointer, then the lifetime is implicit to the smart pointer type. – Klaim Mar 24 '11 at 09:05
  • I'm talking about where you say, "In the end, only if I'm "forced" to I'll use delete in the destructor of the owner class." – That Realtor Programmer Guy Mar 25 '11 at 08:09
  • Typically when you have to manually manage the memory, like in the case of an object pool. You'll then have to carefully manage the new and delete calls, and have a class that just does that. See the following question/answer, this is related to your question and might help understand: http://programmers.stackexchange.com/questions/57581/in-c-is-it-a-reflection-of-poor-software-design-if-objects-are-deleted-manuall/57611#57611 – Klaim Mar 25 '11 at 12:03