Destructor for a List class in C++

Question

I'm new to C++ thus the question. I've a toy implementation of a Singly Linked List in C++.

template<typename T>
class List {
    template<typename U>
    struct Node {
        U data_;
        Node<U>* next_;

        Node() : data_(0), next_(nullptr) {}
        Node(U data) : data_(data), next_(nullptr) {}
    };

private:
    Node<T>* head_;
    std::size_t size_;

public:
    List() : head_{nullptr}, size_{0} {}

    void insert(const T& item) {
        Node<T>* p(new Node<T>(item));
        if (size_ == 0) {
            head_ = p;
        } else {
            p->next_ = head_;
            head_ = p;
        }
        size_++;
    }
    std::size_t getSize() {
        return size_;
    }

    ~List(){
    while(head_){
        Node<T> p = head_;
        delete(p);
        head_ = head_->next_;
    }
};

This code seems to work. The problem though is that the objects allocated by new are never cleaned up, despite the ~List() destructor. Can someone help me understand, how I can write a destructor for this class that cleans up all the allocated nodes ?

Important remark: I am aware that this can be done using smart pointers, but I want to understand the old school way of managing heap.

Answer 1

while(head_){
    Node<T> p = head_; <-- change to pointer
    delete(p); <-- you can't delete this right now
    head_ = head_->next_;
}

p should be a pointer. You cannot delete p right away. You have to find the next node, and delete p later. Also use delete p; instead of delete (p); as follows:

~List() {
    while(head_) {
        Node<T> *p = head_;
        head_ = head_->next_;
        delete p;
    }
}

As noted in comments, Node does not need to be a template. You can simplify your class. insert can also be simplified because head_ is initialized to nullptr, you can safely assign p->next_ = head_;

template<typename T> class List {
    struct Node {
        T data_;
        Node* next_;
        Node() : data_(0), next_(nullptr) {}
        Node(T data) : data_(data), next_(nullptr) {}
    };
    Node* head_;
    std::size_t size_;
public:
    List() : head_{ nullptr }, size_{ 0 } {}

    void insert(const T& item) {
        Node* p = new Node(item);
        p->next_ = head_;
        head_ = p;
        size_++;
    }

    std::size_t getSize() {
        return size_;
    }

    ~List() {
        while(head_) {
            Node *marked = head_;
            head_ = head_->next_;
            delete marked;
        }
    }
};