c++ linked list not working

Question

I am trying to make a linked list but am running into problems because I am deleting my nodes twice. The problem only arises when a node is passed into a function (if it is passed by reference everything is fine) which leads me to believe that the object being passed into the function is being copied in such a way that the pointers are pointing to nodes from the original list not the new list. I tried to get around this by overloading the = operator but this didn't work either. An explanation of what I'm doing wrong would be great.

Thanks for the help

#include <iostream>

struct node{
    node(int n){
        if (n == 1){
            data = 1;
            next = NULL;
        }
        if (n == 2){
            data = 2;
            next = new node(1);
            next -> next = NULL;
        }
    }
    ~node(){
        std::cout << data << std::endl;
        if (next != NULL) delete next;
    }

    void operator=(node a){
        next = NULL;
    }

    int data;
    node* next;
};

void func2(node v){
}


int main(){

    node v(2);
    if (v.next -> next == NULL) std::cout << "true\n";
    func2(v);

    return 0;
}

Answer 1

Your suspicions are correct, but therein lies the problem; when you pass the node into func2, you're only copying the first node, not the entire list. The copy constructor will copy the first node, and the pointer in the first node (which points to the original second node), so when v goes out of scope in func2 it gets deleted once, then gets deleted again when it goes out of scope of main. You'll need to write the copy constructor to do a "deep copy," traversing the entire list and copying every node into a new address.

Remember also that the copy constructor should return *this in most cases, this is in the C++ FAQ and in the book "Effective C++" by Scott Meyers. Thus the signature should be:

 node& operator=(const node& node);

If you're going to overload the assignment operator, you should probably also define a copy constructor. Good job explaining the problem, by the way.

Edit: The code would look like this. I apologize that I haven't tested this; I'm on my tablet and editing this is painful...

#include <iostream>

struct node{
    node(const node& toCopy) : data(toCopy.data) 
    {
        if(toCopy.next != null) {
             next = new node(toCopy);
        }
    }

    node(int n){
        if (n == 1){
            data = 1;
            next = NULL;
        }
        if (n == 2){
            data = 2;
            next = new node(1);
            next -> next = NULL;
        }
    }

   node& operator=(const node& toCopy) {
        if(&toCopy != this) {
             data = toCopy.data;

             if(next != NULL) {
                 next = new node(toCopy);
             }
        }

        return *this;
   }

    ~node(){
        std::cout << data << std::endl;
        if (next != NULL) delete next;
    }

    int data;
    node* next;
};

void func2(node v){
}


int main(){
    node v(2);
    if (v.next -> next == NULL) std::cout << "true\n";
    func2(v);

    return 0;
}