CODEWITHSUNDEEP
c++templateslinked-list
mjr
mjr

Reputation: 141

C++ linked list append method

I am writing a linked list template class in C++ as an exercise for myself to help me get back into C++ programming. I've got the following class definition:

template <typename T>
class List
{
public:
    List();
    ~List();

    void append(T data);
    void display();
    int  count();

private:
    struct Node
    {
        T data;
        Node *next;
    } *head;
};

I have two versions of the append method - one that works and one that doesn't. I can't figure out what the difference, in terms of the operations performed, is, and why the second one doesn't work. Here's the one that works:

template <typename T>
void List<T>::append(T data)
{
    if (head == NULL)
    {
        head = new Node;
        head->data = data;
        head->next = NULL;
    }
    else
    {
        Node *p = head, *q;
        while (p != NULL)
        {
            q = p;
            p = p->next;
        }

        p = new Node;
        p->data = data;
        p->next = NULL;
        q->next = p;
    }
}

And here's the one that doesn't seem to actually add any elements to the list:

template <typename T>
void List<T>::append(T data)
{
    Node *p = head, *q = head;

    while (p != NULL)
    {
        q = p;
        p = p->next;
    }

    p = new Node;
    p->data = data;
    p->next = NULL;
    if (q != NULL)
    {
        q->next = p;
    }
}

Any ideas as to why the second version doesn't add any elements? I've been trying it with type T as int.

P.S. Neither version gives any errors or warnings during compilation, nor during runtime.

Upvotes: 1

Views: 5478

Answers (2)

Ben Voigt
Ben Voigt

Reputation: 283713

The second method only handles the case where the list is non-empty.

When the list is empty, the line q->next = p; is never reached, so the new element is leaked with no pointer existing to it after p goes out of scope.

What you want, if you would like to eliminate the special case for empty list, is a Node **, like thus:

template <typename T>
void List<T>::append(T data)
{
    Node** q = &head; /* head acts as the first Node::next link */

    /* invariant: q points to some Node::next field (or head, which acts like one) */
    while (*q)
        q = &(*q)->next;

    /* invariant: q points to the Node::next field at the end of the chain, which is currently NULL */
    *q = new Node { data, nullptr };
}

Upvotes: 1

littleadv
littleadv

Reputation: 20272

In the first version you change the head, in the second - you don't.

Simpler would be:

template <typename T>
void List<T>::append(T data)
{
    p = new Node;
    p->data = data;
    p->next = head;
    head = p;
}

That would also be more logical because entering an item to a linked list shouldn't take O(n) as it does for you...

If you absolutely have to add to the end, do this:

template <typename T>
void List<T>::append(T data)
{
    p = new Node;
    p->data = data;
    p->next = NULL;
    if (tail)
        tail->next = p;
    else // first time
        tail = head = p;
}

Upvotes: 0

Related Questions