Doubly linked list reimplemented with smart pointers

I need some suggestions on my reimplementation of a doubly linked list with smart pointers since I'm new to smart pointers in C++.

I read some docs on smart pointers and found out there are shared_ptr, unique_ptr and weak_ptr. I only use shared_ptr in my current code, but I'm not sure how to use unique_ptr in my doubly linked list. I would like someone can give me some pointers on how and where to use unique_ptr in my DLL.

```
class Node{
public:
int data;
Node(int data){
this->data = data;
}
public:
shared_ptr prev;
shared_ptr next;
};

class LinkedList{
public:
shared_ptr head;
shared_ptr tail;
public:
LinkedList(){}

void fun(){
printf("fun()\n");
}

void append(shared_ptr node){
if(!head){
head = node;
tail = head;
}else{
tail->next = node;
node->prev = tail;
tail = node;
}
}

void print(){
shared_ptr curr = head;
while(curr){
printf("[%d]\n", curr->data);
curr = curr->next;
}
}
void remove(shared_ptr node){
shared_ptr curr = head;
if(node && curr){
while(curr){
if(curr == node){
if(curr->prev == nullptr){
if(curr->next == nullptr){
// [curr]
node.reset();
head.reset();
tail.reset();
}else{
// [curr]->[]
shared_ptr next = curr->next;
curr->next.reset();
next->prev.reset();
head = next;

Summary

This is obviously a personal opinion, but I don't really like the interface you've defined for your list. By defining your methods in terms of a shared_ptr, you're making the caller responsible for constructing Nodes. It seems like Node should be an implementation detail of your list. Otherwise, since the caller has complete access to the Node they can iterate the list and modify it without the LinkedList class being called at all.

I'd define the interface in terms of the Data that is to be stored, currently this is an int, so you would have something like this:

void append(int newValue)

Or you may want to define your list in terms of a template and take in a pointer of the relevant type. This could be a shared_ptr if ownership of the data doesn't pass solely to your list, or it could be a unique_ptr if the node that references it will be the owner of the Data.

Append

Looking at your current implementation, what happens if a Node is passed in that already has its prev and next pointers populated (with the given interface I could assume that I could insert a Node chain). These pointers aren't checked, they're overwritten. This is part of why I don't like the interface...

Remove

What happens if you're asked to remove a node that isn't in the list? Currently nothing. It feels like this method should either be returning an indicator that the object has been removed, or more likely throwing an exception to indicate that it hasn't..

StackExchange Code Review Q#129715, answer score: 3