What all should be deleted in the destructor of a class

Question

So it's been a while since I've done any c++ coding and I was just wondering which variables in a basic linked list should be deleted in the destructor and unfortunately I can't consult my c++ handbook at the moment regarding the matter. The linked list class looks as follows:

#include <string>
#include <vector>

class Node
{
    Node *next;
    string sName;
    vector<char> cvStuff;

    Node(string _s, int _i)
    {
        next = nullptr;
        sName = _s;

        for (int i = 0; i < _i; i++)
        {
            cvStuff.insert(cvStuff.end(), '_');
        }
    }

    ~Node()
    {
         //since sName is assigned during runtime do I delete?
         //same for cvStuff?
    }
};

I'm also curious, if in the destructor I call

delete next;

will that go to the next node of the linked list and delete that node and thus kind of recursively delete the entire list from that point? Also, if that is the case and I choose for some reason to implement that, would I have to check if next is nullptr before deleting it or would it not make a difference?

Thank you.

Answer 1

Ideally, nothing: use smart pointers: std::unique_ptr<>, std::smart_ptr<>, boost::scoped_ptr<>, etc..

Otherwise, you delete what's an owning native pointer. Is next owning?

• Do you plan to delete something in the middle of the list? If yes, you can't delete in the destructor.
• Do you plan to share tails? If yes, you need reference-counted smart pointers.

It's okay to delete nullptr (does nothing). In the example, you shouldn't delete sName and cvStuff as those are scoped, thus destroyed automatically.

Also, if this is going to be a list that can grow large, you might want to destroy & deallocate *next manually. This is because you don't want to run out of stack space by recursion.

Furthermore, I suggest separating this to List, meaning the data structure and ListNode, meaning an element. Your questions actually show this ambiguity, that you don't know whether you're deleting the ListNode or the List in the destructor. Separating them solves this.

Answer 2

An object with automatic lifetime has it's destructor called when it goes out of scope:

{  // scope
    std::string s;
}  // end scope -> s.~string()

A dynamic object (allocated with new) does not have it's destructor called unless delete is called on it.

For a member variable, the scope is the lifetime of the object.

struct S {
    std::string str_;
    char* p_;
};

int main() {  // scope
    {  // scope
        S s;
    }  // end scope -> s.~S() -> str_.~string()
}

Note in the above that nothing special happens to p_: it's a pointer which is a simple scalar type, so the code does nothing automatic to it.

So in your list class the only thing you have to worry about is your next member: you need to decide whether it is an "owning" pointer or not. If it is an "owning" pointer then you must call delete on the object in your destructor.

Alternatively, you can leverage 'RAII' (resource aquisition is initialization) and use an object to wrap the pointer and provide a destructor that will invoke delete for you:

{  // scope
    std::unique_ptr<Node> ptr = std::make_unique<Node>(args);
}  // end scope -> ptr.~unique_ptr() -> delete -> ~Node()

unique_ptr is a purely owning pointer, the other alternative might be shared_ptr which uses ref-counting so that the underlying object is only deleted when you don't have any remaining shared_ptrs to the object.

You would consider your next pointer to be a non-owning pointer if, say, you have kept the actual addresses of the Nodes somewhere else:

std::vector<Node> nodes;
populate(nodes);

list.insert(&nodes[0]);
list.insert(&nodes[1]);
// ...

in the above case the vector owns the nodes and you definitely should not be calling delete in the Node destructor, because the Nodes aren't yours to delete.

list.insert(new Node(0));
list.insert(new Node(1));

here, the list/Nodes are the only things that have pointers to the nodes, so in this use case we need Node::~Node to call delete or we have a leak.

Answer 3

Basically you should just

delete next 

and that's all you should do:

• The string and vector objects have their own destructors, and since this object is being destructed, theirs will be called.

• Deleting a null pointer is not a problem, so you don't even have to check for that.

• If next is not a null pointer, it will keep on calling the destructors of the next nodes, on and on, as needed.

Just delete it and that's all, then.

Answer 4

Your class destructor will be called (it is empty), then the member objects destructors are called.

If member is not an object, no destructor is called.

In your example:

- List *next: pointer on List: no destructor called
- string sName: string object: destructor called
- vector<char> cvStuff: vector object: destructor called

Good news: you have nothing to do. Destructor declaration is not even useful here.

If you delete next in your destructor, then deleting an item will delete all other items of your list: not very useful.

(and your List object should rather be called Node). The List is the chained result of all your nodes, held by the first node you created.