CODEWITHSUNDEEP
c++
bluewhale
bluewhale

Reputation: 307

Insertion in linked list creating loop

So, I am trying to implement a simple linked list in C++, but I am having a problem with the push method of my class. Basically, when I add the first node to a list, everything goes fine; but, when I add a second node, it ends up pointing to itself (ie, secondNode.next == &secondNode).

class linkedList
{
    public:
    node head;
    linkedList()
    {
        head.next = NULL;
    }
    void push(node new)
    {
        if(head.next == NULL)
        {
            head.next = &new;
            new.next = NULL;
        }
        else
        {
            new.next = head.next;
            head.next = &new;
        }
    }
};

I couldn't figure out what is wrong... Any help would be greatly appreciated.

Upvotes: 0

Views: 812

Answers (2)

Jerry Coffin
Jerry Coffin

Reputation: 490108

At least in my opinion, you have a few things that are wrong to some degree or other.

First, head shouldn't really be a node -- it should be a node *. At least from the looks of things, all you're ever using it for is its next pointer, so you might as well just make it a pointer and be done with it.

Second, to insert new items at the beginning of the list, you don't really need to check for whether the head of the list is a null pointer or not.

Third, although @lezebulon's suggestion to use a reference to a node will work, I don't think it's really the best way to go in this case. Rather than having the user pass a pointer or reference to a node, they should really just pass in a data item, and your linked-list class should allocate a node to hold that item in the list.

template <class T>
class linked_list { 

    class node { 
        T item;
        node *next;
    public:
        node(T const &data, node *next_node) : item(data), next(next_node) {}
    };

    node *head;
public:

    linked_list() : head(NULL) {}

    void push(T const &data) {
        head = new node(data, head);
    }
};

Upvotes: 0

lezebulon
lezebulon

Reputation: 7994

void push(node new)

you have to not make a copy of the object, like so:

void push(node& new)

otherwise you are taking the adress of an object that is deleted at the end of the function

Upvotes: 2

Related Questions