trouble calling delete on pointers in list

[jamesfarrow]

I have created a list class with nodes as an exercise, all 'seems' to be working ok but I can not find a way to free the memory with delete in my destructor...? I'm not sure how/what to call delete om if I'm honest. My class is below.
Thanks in advance!

  #include "list.hpp"

template<typename T>
class list
{
private:
    node<T>* head;
    node<T>* tail;
    
public:
    list(): head(nullptr), tail(nullptr)
    {}
    
    ~list()
    {
        
    }
    
    void push_back(T value) {
        if(!head)
        {
            tail = new node<T>(value);
            head = tail;
        }
        else{
            node<T>* newtail = new node<T>(value);
            tail->next = newtail;
            tail = newtail;
        }
    }
    
    void push_front(T value){
        if(!tail)
        {
            head = new node<T>(value);
            tail = head;
        }
        else
        {
            node<T>* newhead =  new node<T>(value);
            newhead->next = head;
            head = newhead;
        }
    }
    
    void insert(T value, int idx)
    {
        node<T>* previousnode{head};
        node<T>* nextnode{head};
        int node_count{};
        while(node_count < (idx - 1))
        {
            previousnode = previousnode->next;
            node_count++;
        }
        node_count = 0;
        while(node_count < idx)
        {
            nextnode = nextnode->next;
            node_count++;
        }
        node<T>* newtail =  new node<T>(value);
        previousnode->next = newtail;
        newtail->next = nextnode;
    }
    
    void remove(int idx)
    {
        node<T>* first{head};
        node<T>* second{head};
        int node_count{};
        while(node_count < (idx - 1))
        {
            first = first->next;
            node_count++;
        }
        node_count = 0;
        while (node_count < (idx + 1))
        {
            second = second->next;
            node_count++;
        }
        first->next = second;
    }
    
    friend std::ostream& operator<<(std::ostream& out, const list &l)
    {
        const node<T>* temp = l.head;
        while(temp)
        {
            out << temp->value << " ";
            temp = temp->next;
        }
        
        return out;
        
    }
};

#endif /* node_hpp */

[ne555]

void pop_front(){
   //to fix: list is empty, list has only one element
   node<T> *aux = head;
   head = head->next;
   delete aux;
}
~list(){
   while(not empty())
      pop_fron();
}

your `remove()' function is leaking memory

[jamesfarrow]

Thanks! I did wonder about remove() but wasn’t sure. My remove function essentially ‘skips’ a node. How can I go about freeing the memory of the skipped node?

[ne555]

be careful with two things:
* if you need to delete `x', then you need to be able to reach `x'
* if you delete `x', then you may not access it anymore

you have a->b->c and want to remove b to then have a->c

ptr1 = a;
ptr2 = a->next; //b
ptr1->next = ptr2->next; //a->c
delete ptr2

adapt it to your situation

[jamesfarrow]

Excellent - thanks again. I will have a go tomorrow, hopefully I can adapt it and get it to work.
Watch this space!
Cheers!

[jamesfarrow]

One more thought, this may be daft but, is this a cast for smart pointers? Use std::unique_ptr instead ?

[jamesfarrow]

my destructor

//destructor
    ~list()
    {
        while(head)
        {
            std::cout << "destructor" << std::endl;
            node<T>* temp = head;
            head = head->m_next;
            delete temp;
        }
    }

[dhayden]

Your destructor looks good.

insert() isn't right:
- Can't insert at the head of the list
- Won't modify tail if inserting at the end of the list.

Comment your code. Does insert() put the new item before or after then idx'th item? If you want to be able to insert at the head, then it needs to insert before idx.

Line 86: You should never use "l" as a variable name. It's too easy to confuse to 1.

Linked lists can be simplified in C++ with the use of references and the & (address of) operator. Start with a generalized method to insert into the list:

    // Given a value in a reference to "head" or a node's "next" pointer,
    // insert a new node with the value before the one that headOrNext points to.
    void insertBefore (T value, node * &headOrNext) {
        node *newNode = new node(value);
        newNode->next = headOrNext;
        headOrNext = newNode;
        if (tail == nullptr || &tail->next == &headOrNext) {
            tail = newNode;
        }
    }

Now push_back() and push_front() become trivial:

    void push_back(T value) {
        if (tail) {
            insertBefore(value, tail->next);
        } else {
            insertBefore(value, head);
        }
    }

    void push_front(T value){
        insertBefore(value, head);
    }

insert() is a little harder, but only a little, The trick is to use a pointer to the head or next pointer.

    void insert(T value, int idx)
    {
        node **pp;
        for (pp = &head; idx--; pp = &(*pp)->next)
            ;
        insertBefore(value, *pp);
    }

remove() would be just as simple, except for that damn tail pointer.

    void remove(int idx)
    {
        if (idx == 0) {
            node *tmp = head;
            if (head == tail) tail = nullptr;
            head = head->next;
            delete tmp;
            return;
        }

        node *prev;
        for (prev = head; --idx; prev = prev->next)
            ;
        node *tmp = prev->next; // tmp is the node to delete
        prev->next = tmp->next;
        if (tail == tmp) {
            tail = prev;
        }
        delete tmp;
    }

[jamesfarrow]

Thanks everyone all seems well!