What is the proper way to delete a singly-linked-list in destructor?

Question

Hello I have a bit ambiguity about writing a proper destructor:

class SLLst 
{
public:
    SLLst() = default;
    SLLst(const SLLst&);
    SLLst& operator=(SLLst);
    ~SLLst();

    void insert(int);
    void remove(int);

private:
    SLLst* next = nullptr;
    int data = 0;
    friend void swap(SLLst&, SLLst&);
    friend std::ostream& print(std::ostream&, const SLLst&);
};

SLLst::SLLst(const SLLst& rhs) : 
    next(rhs.next ? new SLLst() : nullptr),
    data(rhs.data)
{
    cout << "cpy-ctor" << endl;
}

SLLst& SLLst::operator=(SLLst rhs)
{
    cout << "operator=(SLLst)" << endl;
    using std::swap;
    swap(*this, rhs);
    return *this;
}

void swap(SLLst& lhs, SLLst& rhs)
{
    cout << "operator=(SLLst)" << endl;
    using std::swap;
    swap(lhs.next, rhs.next);
    swap(lhs.data, rhs.data);
}

SLLst::~SLLst()
{
    cout << "dtor" << endl;
    delete next;// is this enough?

    // or should I use this code?
    //SLLst* cur = next;
    //SLLst* n = nullptr;

    //while (cur != NULL) {
    //  n = cur->next;
    //  cur->next = nullptr;
    //  delete cur;
    //  cur = n;
    //}

}


void SLLst::insert(int x)
{
    SLLst* tmp = new SLLst();
    tmp->data = x;
    if (!next)
    {
        next = tmp;
        return;
    }

    tmp->next = next;
    next = tmp;
}

std::ostream& print(std::ostream& out, const SLLst& lst)
{
    auto tmp = lst.next;
    while (tmp)
    {
        out << tmp->data << ", ";
        tmp = tmp->next;
    }
    return out;
}

As you can see if I just use delete next; in destructor then I get it called as many as nodes in the list but why many implementations use a loop to free the nodes like the commented code in destructor?

• Because if I only call delete on next then the destructor will be called recursively thus I think I don't need a loop to free the nodes in destructor? is it correct?

• When should I use a loop to free nodes in destructor? Thank you!

*If I run my code I'll get:

81, 77, 57, 23, 16, 7, 5,

done
dtor
dtor
dtor
dtor
dtor
dtor
dtor
dtor

• As you can see dtor is called 8 times; does this mean it has properly freed all the nodes?

Answer 1

As you can see if I just use delete next; in destructor then I get it called as many as nodes in the list

Yep.

Because if I only call delete on next then the destructor will be called recursively thus I think I don't need a loop to free the nodes in destructor? is it correct?

Yep.

When should I use a loop to free nodes in destructor?

When you need to.

but why many implementations use a loop to free the nodes like the commented code in destructor?

Because many implementations are "C-like", and do not use destructors. So they need to.

You are making the most of C++'s object management features to "do the loop for you". Yay!

(Although, to be honest, I would still do it in a loop, because your way is potentially quite stack-heavy.)

Now go one step further and switch to std::list (or std::forward_list).

Answer 2

Just immediately off the bat, std::unique_ptr would take care of this for you, but if you want to do it yourself, then it would look something like this, keep in mind this is a minimal example.

RAII is a very important principle in C++, it basically means, allocate when you need it, but destroy whenever you're done using it.

So if a node is being pointed to, and you delete the node that points to it, then that node should also destroy the thing it points to since it has ownership over it.

class List {
    Node* first_node;

    ~List() {
        delete first_node;
    }

};

class Node {
    ~Node() {
        delete next; // will then in turn destroy the one it points to untill one is nullptr, deleting nullptr is well defined in C++ nowadays
    }
    Node* next;

};

Example with std::unique_ptr

class List {
    std::unique_ptr<Node> first_node;

    // default dtor
};

class Node {
    std::unique_ptr<Node> next;
    // default dtor

};