1

I've the following singly linked list implementation.

template <typename T> struct node{
    node(T data):data(data),next(nullptr){}
    T data;
    node<T> * next;
};

template< typename T> class slist{
    node<T>* head;
    int size;
public:
    slist(node<T>* head):head(head), size(0){}

    slist(const slist<T>& rhs){

        node<T>* temp = rhs.getHead();
        node<T>* p = new node<T>(temp->data);
        head = p;
        node<T>* current = p;
        while(temp != nullptr){
            current = current->next;
            current->data = temp->data;
        }

    }
    ~slist(){
        if(head == nullptr) return;

        while(head != nullptr){
            node<T>* current = head;
            head = head->next;
            delete(current);
        }
    }
    slist& operator= (const slist& rhs){


    }
    node<T>* getHead()const {
        return head;
    }


    void insertFront(T item){
        node<T>* p = new node<T>(item);
        if(head == nullptr){
            p = head;
            size++;
            return;
        }
        p->next = head;
        head = p;
        size++;
    }

    void insertBack(T item){
        node<T>* p = new node<T>(item);
        node<T>* current = head;
        while(current->next != nullptr){
            current = current->next;
        }
        current->next = p;
        size++;
    }

    void remove(T item){
        bool check = false;

        node<T>* current = head;
        try {
            while(current != nullptr){
                if(current->data == item) check = true;
                current = current->next;
            }
            if(!check){
                throw std::runtime_error("Item not in list");

            }
        }catch(std::runtime_error& e){
            std::cout<<e.what()<<std::endl;
            exit(-1);
        }

        current = head;
        while(current != nullptr){
           if(current->next->data == item){
               node<T>* temp = current->next;
               current->next = current->next->next;
               delete(temp);
               break;
           }
            current = current->next;
        }
        size--;

    }
    int getSize () const {
        return size;
    }

    void printList(){
        node<T>* current = head;
        while(current != nullptr){
            if(current->next != nullptr){
                std::cout<<current->data<<"->";
            }else{
                std::cout<<current->data<<std::endl;
            }
            current = current->next;
        }
    }


};

Based on the current implementation of the class and the copy constructor, can someone help with the assignment operator overload. Also I'm a little confused about the copy constructor and assignment overload. What I understand is that the copy constructor creates a new list which has the same values as the old list from a old list. This the next address for all nodes will be different but the values will be the same since it's a deep copy. Is my understanding correct and then onwards what does the assignment overload do?

Antithesis
  • 327
  • 2
  • 4
  • 17
  • Copy assignment has the same goal as copy construction. You just need to be aware you had data already in the container. A copy constructor should create a container that has the same data as the original container, but is an independent object. – Weak to Enuma Elish Nov 22 '15 at 18:57
  • 2
    Use the copy-and-swap idiom (google it). – n. m. could be an AI Nov 22 '15 at 18:59
  • In your copy constructor, you need to create every node with `new`, not just the first one. (There should be a `new` inside the loop.) – molbdnilo Nov 22 '15 at 19:01

2 Answers2

5

If you already have a copy constructor and destructor, also implement swap() and then implement your copy assignment operator in terms of these three, e.g.:

template <typename T>
slist<T>& slist<T>::operator= (slist<T> other) {
    this->swap(other);
    return *this;
}

Note that the argument is readily copied: unlike the copy constructor the copy assignment can take its argument by value.

With respect of the semantics:

  • The copy constructor creates a new object with the same value as the original, e.g.:

    slist<int> sl1;
    // insert elements into sl1
    slist<int> sl2(sl1); // uses the copy constructor
    
  • The copy assignment replaces the value of ane existing object the value of the assigned value, e.g.

    slist<int> sl1;
    slist<int> sl2;
    // possibly modify both sl1 and sl2
    sl2 = sl1; // uses the copy assignment operator
    
Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
1

You need to consider these cases:

  • copy construction, i.e. slist<int> newone = other;
  • copy assignment, i.e. slist<int> a; /* ... */ a = other;

Now, both of these operations make - as the name suggests - a copy of the original data structure. What exactly that means depends on how you want to implement it, but the following should - to stay close to the principle of least surprise - hold:

slist<int> a = some_generator(), b = another_generator();
slist<int> c = a;
// a == c should be true
b = a;
// a == b should be true now
modify(a);
// a == b should be false now, BUT b should be in the same state as before!

The easiest way to achieve this is by making - as you already suggested - deep copies. Thus you basically do the same as in your copy constructor. You make copies of each node, such that they have the same value as the original nodes, but are different entities.

If you're also targeting "modern C++", that is C++11 an onwards, then there's also a move constructor and a move assignment operator that you might want to implement.


As mentioned in the comments, your deep copy algorithm isn't correct: You need to make a copy of each node:

// if in assigment, delete the nodes pointed to by head first!
node<T> const * iterator = other.getHead();
if (iterator != nullptr) { // in your implementation, head could be a nullptr
  node<T> * new_node = new node<T>(*iterator); // make a copy of head
  head = new_node;
  while (iterator->next) {
    iterator = iterator->next;
    node<T> * copy = new node<T>(*iterator);
    new_node->next = copy;
    new_node = copy;
  }
  new_node->next = nullptr;
}

Also, if you can, prefer smart pointers, unique_ptr in this case, instead of raw pointers.

Daniel Jour
  • 15,896
  • 2
  • 36
  • 63