What am I doing wrong with std::shared_ptr and std::unique_ptr in my linked list implementation?

Question

I've never used smart pointers before, so I decided to try out implementing a basic little linked list, just to see how it works. The program bellow outputs only the first element of the list, i.e. 5, and then just exits.

In the print() function, the while loop iterates only once, which means that the list contains only one element even though it should contain 3.

Here's the code:

#include <iostream>
#include <memory>

class list {
private:
    struct node {
        int val;
        std::shared_ptr<node> next;
        node(int _val) : val(_val), next(nullptr) {}
    };
    std::shared_ptr<node> head;

public:
    list() {
        head = nullptr;
    }

    void push_back(int val) {
        std::unique_ptr<node> new_node(new node(val));
        if(head == nullptr) {
            head = std::move(new_node);
        } else {
            std::shared_ptr<node> curr(head);
            while(curr != nullptr) {
                curr = curr->next;
            }
            curr = std::move(new_node);
        }
    }

    void print() {
        std::shared_ptr<node> curr(head);
        while(curr != nullptr) {
            std::cout << curr->val << " ";
            curr = curr->next;
        }
        std::cout << std::endl;
    }
};

int main() {
    std::unique_ptr<list> lst(new list());

    lst->push_back(5);
    lst->push_back(10);
    lst->print();

    return 0;
}

score 3 · Accepted Answer · answered Oct 09 '13 at 16:02

You are not appending the node to the end correctly. This part of your code:

while(curr != nullptr) {
    curr = curr->next;
}
curr = std::move(new_node);

should instead be:

while(curr->next != nullptr) {
    curr = curr->next;
}
curr->next = std::move(new_node);

What am I doing wrong with std::shared_ptr and std::unique_ptr in my linked list implementation?

1 Answers1