CODEWITHSUNDEEP
c++c++11linked-listcopy-constructor
bigZigZag
bigZigZag

Reputation: 33

Copy Constructor is not working for linked list?

The following is the class with the node structure, the linked list copy constructor, and my main file. It is printing the numbers in the first list and it only copies the first number (15) into the second list. It calls the destructors for both lists and the program closes properly. I cannot figure out this copy constructor no matter how hard I try and it is really starting to bother me.

#ifndef LINKEDLIST_H
#define LINKEDLIST_H
#include <iostream>
using namespace std;

// Declare a class for the list
class LinkedList
{
private:
    // Declaring a structure for the list
    struct ListNode
    {
        // Integer value in the node and pointer to the next node
        int value = NULL;
        struct ListNode* next = nullptr;
    };

    // List head pointer
    ListNode* head = nullptr;
public:
    // Constructor
    LinkedList() { head = nullptr; };

    // Copy constructor
    LinkedList(const LinkedList &);

    // Destructor
    virtual ~LinkedList();

    // Linked list operations
    void appendNode(int);
    void insertNode(int);
    void deleteNode(int);
    void printList() const;
    void reverseList() const;
    int searchList(const int) const;
};

#endif // LINKEDLIST_H

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* newNode = nullptr; // Create new node
    ListNode* copyPtr = nullptr; // Point to original object

    copyPtr = listObj.head;

    newNode = new ListNode;
    this->head = newNode;
    head->value = copyPtr->value;
    copyPtr = copyPtr->next;

    while(!copyPtr)
    {
        newNode->next = new ListNode;
        newNode = newNode->next;
        newNode->value = copyPtr->value;
        copyPtr = copyPtr->next;
    }
}

#include "LinkedList.h"
#include <iostream>
using namespace std;

int main()
{
    LinkedList list1;
    list1.appendNode(15);
    list1.appendNode(20);
    list1.appendNode(25);

    list1.printList();

    LinkedList list2 = list1;

    list2.printList();

    return 0;
}

Upvotes: 0

Views: 333

Answers (3)

bigZigZag
bigZigZag

Reputation: 33

Thank you guys for the help! Changing to to while(copyPtr) made it work. Such a silly mistake. I really appreciate it.

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* newNode = nullptr; // Create new node
    ListNode* copyPtr = nullptr; // Point to original object

    if(!listObj.head)
        return;
    else
    {
        copyPtr = listObj.head;

        newNode = new ListNode;
        this->head = newNode;
        head->value = copyPtr->value;
        copyPtr = copyPtr->next;

        while(copyPtr)
        {
            newNode->next = new ListNode;
            newNode = newNode->next;
            newNode->value = copyPtr->value;
            copyPtr = copyPtr->next;
            newNode->next = nullptr;
        }
    }
}

Upvotes: 0

wanghongyun
wanghongyun

Reputation: 81

I find two problems with your code:

  1. You don't check whether listObj is nullptr.
  2. With the end of while loop, you don't set newNode->next to nullptr.

Upvotes: 1

Barry
Barry

Reputation: 302817

Your loop condition is incorrect. You want to loop while there is a next node. You are doing the opposite:

while(!copyPtr)

Should be:

while (copyPtr)

Note also that your copy constructor will be incorrect if the list you're copying from is empty. You dereference listObj.head before you do any checking.

Upvotes: 3

Related Questions