When should I provide a destructor for my class?

Question

This seems like a rather trivial or at least common question, but I couldn't find a satisfying answer on google or on SO.

I'm not sure when I should implement a destructor for my class.

An obvious case is when the class wraps a connection to a file, and I want to make sure the connection is closed so I close it in the destructor.

But I want to know in general, how can I know if I should define a destructor. What guidelines are there that I can check to see if I should have a destructor in this class?

One such guideline I can think of, is if the class contains any member pointers. The default destructor would destory the pointers on deletion, but not the objects they're pointing at. So that should be the work of a user-defined destructor. E.g: (I'm a C++ newbie, so this code might not compile).

class MyContainer {
public:
    MyContainer(int size) : data(new int[size]) { }
    ~MyContainer(){
        delete [] data;
    }
    // .. stuff omitted
private:
    int* data;
}

If I hadn't supplied that destructor, than destroying a MyContainer object would mean creating a leak, since all the data previously referenced by data wouldn't have been deleted.

But I have two questions:

1- Is this the only 'guideline'? I.e. define a destructor if the class has member pointers or if it's managing a resource? Or is there anything else?

2- Are there cases when I should not delete member pointers? What about references?

Answer 1

You need to define a destructor if the default destruction does not suffice. Of course, this just punts the question: what does the default destructor do? Well, it calls the destructors of each of the member variables, and that's it. If this is enough for you, you're good to go. If it's not, then you need to write a destructor.

The most common example is the case of allocating a pointer with new. A pointer (to any type) is a primitive, and the destructor just makes the pointer itself go away, without touching the pointed to memory. So the default destructor of a pointer does not have the right behavior for us (it will leak memory), hence we need a delete call in the destructor. Imagine now we change the raw pointer to a smart pointer. When the smart pointer is destroyed, it also calls the destructor of whatever its pointing to, and then frees the memory. So a smart pointer's destructor is sufficient.

By understanding the underlying reason behind the most common case, you can reason about less common cases. It's true that very often, if you're using smart pointers and std library containers, their destructors do the right thing and you don't need to write a destructor at all. But there are still exceptions.

Suppose you have a Logger class. This logger class is smart though, it buffers up a bunch of messages to Log, and then writes them out to a file only when the buffer reaches a certain size (it "flushes" the buffer). This can be more performant than just dumping everything to a file immediately. When the Logger is destroyed, you need to flush everything from the buffer regardless of whether it's full, so you'll probably want to write a destructor for it, even though its easy enough to implement Logger in terms of std::vector and std::string so that nothing leaks when its destroyed.

Edit: I didn't see question 2. The answer to question 2 is that you should not call delete if it is a non-owning pointer. In other words, if some other class or scope is solely responsible for cleaning up after this object, and you have the pointer "just to look", then do not call delete. The reason why is if you call delete and somebody else owns it, the pointer gets delete called on it twice:

struct A {
  A(SomeObj * obj) : m_obj(obj){};
  SomeObj * m_obj;
  ~A(){delete m_obj;};
}

SomeObj * obj = new SomeObj();
A a(obj);
delete obj; // bad!

In fact, arguably the guideline in c++11 is to NEVER call delete on a pointer. Why? Well, if you call delete on a pointer, it means you own it. And if you own it, there's no reason not to use a smart pointer, in particular unique_ptr is virtually the same speed and does this automatically, and is far more likely to be thread safe.

Further, furthermore (forgive me I'm getting really into this now), it's generally a bad idea to make non-owning views of objects (raw pointers or references) members of other objects. Why? Because, the object with the raw pointer may not have to worry about destroying the other object since it doesn't own it, but it has no way of knowing when it will be destroyed. The pointed to object could be destroyed while the object with the pointer is still alive:

struct A {
  SomeObj * m_obj;
  void func(){m_obj->doStuff();};
}

A a;
if(blah) {
  SomeObj b;
  a.m_obj = &b;
}
a.func() // bad!

Note that this only applies to member fields of objects. Passing a view of an object into a function (member or not) is safe, because the function is called in the enclosing scope of the object itself, so this is not an issue.

The harsh conclusion of all this is that unless you know what you're doing, you just shouldn't ever have raw pointers or references as member fields of objects.

Edit 2: I guess the overall conclusion (which is really nice!) is that in general, your classes should be written in such a way that they don't need destructors unless the destructors do something semantically meaningful. In my Logger example, the Logger has to be flushed, something important has to happen before destruction. You should not write (generally) classes that need to do trivial clean-up after their members, member variables should clean up after themselves.