Push_front and push_back methods for linked list seems to remove a node

Question

I'm writing a class called Playlist which performs different operations on PlaylistNodes. I looked online and tried to implement push_back and push_frontmethods, but I was unsuccessful.

PlaylistNode *PlaylistNode::insert_next(PlaylistNode *p) {
    PlaylistNode *tmp = nullptr;

    tmp = this->next;
    this->next = p;
    p->next = tmp;

    return p;
}

Playlist::Playlist() {
    head = new PlaylistNode;
    prevToCurr = head;
    tail = head;
    size = 0;
}

Playlist *Playlist::push_back(PlaylistNode *p) {
    PlaylistNode *tmp;

    tmp = tail;
    tmp->insert_next(p);
    tail = p;

    prevToCurr = tail;

    size++;
    return this;
}

Playlist *Playlist::push_front(PlaylistNode *p) {
    size++;

    PlaylistNode *tmp = head;
    head = p;
    head->insert_next(tmp);

    return this;
}

When I run :

play.push_front(node1);
play.push_front(node2);
play.push_front(node2);

then print the linked list, I only get 2 nodes:

ID 44: song2
ID 33: song1

Also, unless it's an exercise on data structures, I suggest using `std::vector` (or `std::list`, if you really need a linked list) instead of a custom linked list. It'd make your life a lot easier. — HolyBlackCat, Jul 08 '19 at 18:36
if you push node2 twice, you will corrupt its 'next' pointer when you do it second time. — Serge, Jul 08 '19 at 18:58
Why are you surprised that you only get two nodes when you only *have* two nodes? — David Schwartz, Jul 08 '19 at 19:06

score 1 · Answer 1 · answered Jul 08 '19 at 19:21

Your initialization method (constructor) is not doing what it's supposed to. When constructing this type of list, head and tail must both point to null as the list is empty. I'm not sure what "prevToCurr" does but I don't think lists use something like that so I would get rid of it:

Playlist::Playlist() {
    head = null;
    tail = null;
    size = 0;
}

For simplicity, Handle push_front() with 2 cases: when the list is empty and when the list has nodes.

If the list is empty all you have to do is make head and tail point to the new node.
When the list is not empty, you make the previous head point to the new node and update the list head.

Would look like this:

Playlist *Playlist::push_front(PlaylistNode *p) {
    if (size == 0) {
        head = p;
        tail = p;
    }
    else {
        head->insertNext(p);
        head = p;
    }
    size++;
    return this;
}

Your push_back() method would be implemented almost the same way as the push_front, updating tail instead of head and making the new tail point to the old one, I'm sure you can figure it out.

You don't give enough info so I'm going to guess your PlaylistNode::insertNext sets the next node. In this case, what you do here is set the next pointer to the node provided as argument:

PlaylistNode *PlaylistNode::insert_next(PlaylistNode *p) {
    this->next = p;
    return this;
}

That should more or less work, as long as you're creating your PlaylistNodes correctly (don't push the same node 2 times, you will end up with a node pointing next to itself as stated in the comments).

score 0 · Answer 2 · answered Jul 08 '19 at 18:54

As noted in the comments this is hard to figure out without all the code, so we have to infer. Working backwards from what could cause this, we assume that the node passed to push_front has a null next pointer. Then your insert_next function will take that new node's null pointer and incorrectly update the inserted node's next pointer to null regardless of what it was before.

Push_front and push_back methods for linked list seems to remove a node

2 Answers2