0

I've been building on a template Linked List and recently implemented a user-defined copy constructor to handle deep copies of my Linked List. My current solution compiles just fine but when I execute, nothing is displayed.

#include <iostream>

template<typename T>
class LinkedList
{
private:
    struct Node
    {
        T data;
        Node* next;
    };

    Node* head;
    Node* tail;
    int size;

public:
    LinkedList();
    LinkedList(const LinkedList<T> &list);
    ~LinkedList();
    LinkedList<T>& operator=(const LinkedList<T> list);
    int getSize() const;
    void display() const;
    void push(const T &data);
    void insert(const int pos, const T &data);
    void remove(const int pos);

};

template<typename T>
LinkedList<T>::LinkedList() : head{ nullptr }, tail{ nullptr }, size{ 0 }
{
}

// TODO: User-defined copy constructor
template<typename T>
LinkedList<T>::LinkedList(const LinkedList<T> &list)
    : head{ nullptr }, tail{ nullptr }, size{list.size}
{
    std::cout << "In the copy constructor\n";
    if (list.head == nullptr)
    {
        return;
    }
    else
    {
        Node* curNode = new Node{list.head->data, nullptr};
        head = curNode; // sets the head member variable to first node in Linked List

        Node* curListNode = list.head;      

        while (curListNode->next != nullptr)
        {
            curNode->next = new Node{curListNode->next->data, nullptr};

            curListNode = curListNode->next;
            curNode = curNode->next;
        }

        curNode->next = new Node{curListNode->next->data, nullptr};
        tail = curNode->next;
    }

}

template<typename T>
LinkedList<T>::~LinkedList()
{
    Node* prevNode = head;
    Node* curNode = head;

    while(curNode != nullptr)
    {
        prevNode = curNode;
        curNode = curNode->next;

        delete prevNode;
    }

    head = nullptr;
    tail = nullptr;
}

template<typename T>
LinkedList<T>& LinkedList<T>::operator=(const LinkedList<T> list)
{
    // make a copy of each node - much like the copy constructor
    std::cout << "In the overloaded assignment\n";
}

template<typename T>
int LinkedList<T>::getSize() const
{
    return size;
}

template<typename T>
void LinkedList<T>::display() const
{
    Node* curNode = head;

    while (curNode != nullptr)
    {
        std::cout << curNode->data << '\n';
        curNode = curNode->next;
    }

    std::cout << '\n';
}

template<typename T>
void LinkedList<T>::push(const T &data)
{
    Node* newNode = new Node{data, nullptr};

    if (size == 0)
    {
        head = newNode;
        tail = newNode;
    }
    else 
    {
        tail->next = newNode;
        tail = newNode;
    }

    ++size;
}

template<typename T>
void LinkedList<T>::insert(const int pos, const T &data)
{
    if (pos < 0 || pos > size)
    {
        throw "Index is out of range!";   
    }
    else if (pos == size)
    {
        push(data);   
    }
    else
    {
        Node* newNode = new Node{data, nullptr};
        Node* prevNode = head;
        Node* curNode = head;

        int i = 0;
        while (i != pos)
        {
            prevNode = curNode;
            curNode = curNode->next;
            ++i;
        }

        prevNode->next = newNode;
        newNode->next = curNode;

        ++size;
    }
}

template<typename T>
void LinkedList<T>::remove(const int pos)
{
    if (pos < 0 || pos > size)
    {
        throw "Index is out of range!";   
    }
    else if (size == 0)
    {
        throw "List is empty!";   
    }
    else
    {
        Node* prevNode = head;
        Node* curNode = head;

        int i = 1;
        while (i != pos)
        {
            prevNode = curNode;
            curNode = curNode->next;
            ++i;
        }

        prevNode->next = curNode->next;
        delete curNode;

        if (pos == size)
        {
            tail = prevNode;   
        }

        --size;
    }
}

int main()
{
    LinkedList<int> list;

    list.push(1);
    list.push(3);
    list.push(6);

    list.display();

    LinkedList<int> copyList{list};

    copyList.display();

    return 0;
}

The first LinkedList<int> list doesn't even display when I add the the copyList statement to main(). However, when I comment out the two copyList statements, list displays as normal.

I added print statements to see if either the copy constructor or the operator= functions were being called and neither of those are printed to the console when the copyList statements are added to my program (the code compiles just fine).

I could use any help to help diagnose what's causing this behavior.

  • 2
    First off, you should not rely on side-effects within copy constructors, because copy-elision is a thing. Copy-assignment should not be elided, though. /OT: `I've been building on a template Linked List` I appreciate that you are most likely doing this as an exercise, whether for a class or your own research, but ideally one should never need to implement something like this in C++ because it has a rich Standard Library that should provide such common things out-of-the-box, in this case as `std::list` (unless you have extremely specific requirements and have determined that to be a bottleneck. – underscore_d Jun 21 '17 at 15:58
  • Could you elaborate on what you mean by side-effects? Completely agree with your comment regarding the utilization of `std::list`, I'm pursuing this purely as an academic exercise. –  Jun 21 '17 at 16:00
  • 1
    Side effects: a copy-constructor doing anything that could be observable outwith itself. See https://stackoverflow.com/questions/28659879/copy-elision-visible-side-effect – underscore_d Jun 21 '17 at 16:01

1 Answers1

0

A simple copy constructor like this one should suffice:

template<typename T>
LinkedList<T>::LinkedList(const LinkedList<T> &list)
    : head{ nullptr }, tail{ nullptr }, size{0}
{
    std::cout << "In the copy constructor\n";
   Node* curNode = list.head;

    while (curNode != nullptr)
    {
      this->push(curNode->data);
        curNode = curNode->next;
    }

}

Note that this is just your display method rewritten to insert elements in the current list.

Jean-Baptiste Yunès
  • 34,548
  • 4
  • 48
  • 69
  • Thanks, this works very well. Would you happen to have any insight into why my implementation would prevent the call to the copy constructor from happening (or, why I wouldn't see my string message when the copy constructor is called). –  Jun 21 '17 at 16:08
  • 1
    On my machine it crashes because you get out of the loop with condition `curListNode->next != nullptr` being false, then you executes `curNode->next = new Node{curListNode->next->data, nullptr};` after that everything is possible as `curListNode->next` is null. – Jean-Baptiste Yunès Jun 21 '17 at 16:11
  • I see - thank you. I'm currently using cpp.sh in my browser and it must not be able to handle such a thing. –  Jun 21 '17 at 16:13
  • 1
    You may correct the code by setting `curNode->next = nullptr; tail = curNode;` just after the loop. – Jean-Baptiste Yunès Jun 21 '17 at 16:13