Unable to grow my linked list

Question

Given below is my code snippet for linked list. I am not able to add number. Whenever I try to add to my list, numbers are getting replaced, so my list never grows. Can you please let me know what is wrong in this code. It would also be very helpful if you could comment on my coding fashion.

   using namespace std;

struct node
{
  int number;
  std::shared_ptr<node> next;
};


bool isEmpty(std::shared_ptr<node> &head)
{
 return (head == NULL);
}


void add(std::shared_ptr<node> &head, int number)
{
 std:: shared_ptr<node> temp;
 temp.reset(new node);

 //temp = head; 

 cout<<"\n Adddress of head: "<<head.get();
// cout<<"\nAddress of temp: "<<temp.get();
 if(isEmpty(head))
 {
   head.reset(new node);
   head->number = number;
   head->next = NULL;
   cout<<"\nAdded first element";  
  }

  else
  {
     cout<<"\nAdding element to exisiting list";
     while(head->next!= NULL)
       { 
        cout<<"\n traversing to next element----->"<<temp->number;
    head = head->next;
       }     

     shared_ptr<node> newNode; 
     newNode.reset(new node);
     newNode->number = number;
     newNode->next = NULL;
     head->next = newNode;
     cout<<"\n address of newNode: "<<newNode.get();

   //  head->next = temp;      
  }   
  //cout<<"\nExiting add";
}



int main()
{
  int number;
  std::shared_ptr<node> head(nullptr);  
   char choice;
  add(head, number);

 return 0;
}

Answer 1

Why are you using reset on shared pointer?

http://en.cppreference.com/w/cpp/memory/shared_ptr/reset

Check the following example:

https://codereview.stackexchange.com/questions/33136/singly-linked-list-with-smart-pointers

Answer 2

Let's take a walk through the add function. I've tweaked the indentation for easier reading.

void add(std::shared_ptr<node> &head, int number)
{
    std::shared_ptr<node> temp;
    temp.reset(new node);

What are you using temp for? Nothing that I can see.

    //temp = head;

    cout << "\n Adddress of head: " << head.get();
// cout<<"\nAddress of temp: "<<temp.get();
    if (isEmpty(head))
    {
        head.reset(new node);
        head->number = number;
        head->next = NULL;
        cout << "\nAdded first element";
    }

OK. That case looks good.

    else
    {
        cout << "\nAdding element to exisiting list";
        while (head->next != NULL)
        {
            cout << "\n traversing to next element----->" << temp->number;
            head = head->next;

Whups! Just moved the head. You just lost that first node element. No one points at it anymore. Shared pointer prevents the leak by destroying it for you. Nice, but you've still lost the data. Fortunately head points to the former head->next, preventing destruction when the former head goes to its grave with next. Shared pointer saves your bacon, but injects a load of extra overhead.

        }

        shared_ptr<node> newNode;
        newNode.reset(new node);
        newNode->number = number;
        newNode->next = NULL;
        head->next = newNode;
        cout << "\n address of newNode: " << newNode.get();

        //  head->next = temp;
    }
    //cout<<"\nExiting add";
}

On the coding style front, I would use a Linked List class to make this a bit easier to deal with:

#include <iostream>
class LinkedList // Ahhhr. Here be the class
{
    struct node // node is a private member of the class hidden away from sight.
    {
        int number;
        node * next;
    };
    node * head; // no shared pointer. We'll handle the memory ourselves.

public:
    LinkedList(): head(nullptr) // construct and init LinkedList
    {

    }

    // we need a copy constructor to be Rule of Three compliant
    LinkedList(const LinkedList & src): head(nullptr) // copy constructor
    {
        node * cur = src.head;
        while (cur != nullptr)
        {
            add(cur->number);
            cur = cur->next;
        }
    }
    ~LinkedList() // free up the nodes
    {
        while (head->next != nullptr)
        {
            node *temp = head;
            head = head->next;
            delete temp;
        }
        delete head;

    }

    // Need assignment operator to be Rule of Three compliant
    // OK this looks a bit weird. src is passed by reference which will 
    // trigger the copy constructor above to do the copy for us. Then we 
    // steal the head from the copy and null it so when src goes out of 
    // scope the destructor doesn't kill all the nodes we just took.
    // This is called the Copy-and-Swap idiom.
    LinkedList & operator=(LinkedList src)
    {
        head = src.head;
        src.head = nullptr;
        return *this;
    }

    bool isEmpty() // essentially unchanged. head is now a class member. 
                   // No need for parameter
    {
        return (head == NULL);
    }

    void add(int number)
    {
        // removed dead code

        if (isEmpty())
        {
            head = new node;
            head->number = number;
            head->next = NULL;
        }

        else
        {
            node * cur = head; // updates a temporary, not the head.
            while (cur->next != NULL)
            {
                cur = cur->next;
            }

            cur->next = new node;
            cur->next->number = number;
            cur->next->next = NULL;
        }
    }
};

// and a quick tester
int main()
{
    LinkedList list;
    list.add(1);
    list.add(2);
    list.add(3);

    return 0;
}

This allows the Linked List to easily be templated and saves trouble later.

More on the Rule of Three and Copy-and-Swap