Linked List copy constructor PROGRAM CRASHING

[bigzigzag]

My program is compiling but it crashes when executing this and I cannot figure out what the problem is. Any help??

Thank you!

#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;
        struct ListNode* next;
    };

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

    // Copy constructor
    LinkedList(LinkedList &);

    // Destructor
    virtual ~LinkedList();

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

#endif // LINKEDLIST_H

LinkedList::LinkedList(LinkedList& listObj)
{
    ListNode* nodePtr = this->head;
    ListNode* copyPtr = listObj.head;

    nodePtr->value = copyPtr->value;
    nodePtr->next = nullptr;

    if(!head)
        head = copyPtr;

    while(copyPtr->next != nullptr)
    {
        appendNode(copyPtr->value);
        copyPtr = copyPtr->next;
    }
}

[alex067]

Just by glancing at it, why is the constuctor accepting an argument of const object? Constructor actually changes the values, you are placing value of copyPtr to the node and changing the next pointer to null.

[bigzigzag]

I took the const out and have the same problem.

[alex067]

try passing LinkedList *&

[MikeyBoy]

@bigzigzag:

Have you tried stepping through it with a debugger to see where it crashes, and what the state of the memory is at that point?

Are you certain that nodePtr and copyPtr aren't null?

@alex067:

I don't understand your comment. The copy constructor doesn't change the value of listObj, so why would it not be declared const?

[bigzigzag]

@MikeyBoy

I don't see how they could be null if I initialized both of them. My code in main.cpp:

#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;
}

list1 works and list2 causes the program to crash when it is instantiated and it's initialized with list1

[ne555]

> I don't see how they could be null if I initialized both of them.
¿where? your default constructor makes `head' null so if you do

LinkedList foo;
LinkedList bar(foo);

it should be obvious that it would fail

In your code you use `.appendNode()' but you don't provide its definition.

LinkedList::LinkedList(LinkedList& listObj)
{
    ListNode* nodePtr = this->head;

you never initialized this->head, so it holds garbage.

   ListNode* copyPtr = listObj.head;
//...
    if(!head)
        head = copyPtr; //head = listObj.head 

that's a shallow copy

[bigzigzag]

@ne555

I changed it to this and still have the same problem

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* nodePtr = nullptr;
    ListNode* newNode = new ListNode;
    ListNode* copyPtr = listObj.head;

    newNode->value = copyPtr->value;
    newNode->next = nullptr;

    if(!head)
        head = listObj.head;

    while(copyPtr->next != nullptr)
    {
        appendNode(copyPtr->value);
        copyPtr = copyPtr->next;
    }
}

[bigzigzag]

I changed it to this and the program is not crashing anymore but it is still not giving me the correct output. The output is giving me 15 20 25 15 and it should be 15 20 25 15 20 25.

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* newNode = nullptr;
    ListNode* copyPtr = listObj.head;

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

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

[bigzigzag]

This is my current copy constructor. It copies the nodes and the program displays them but the program does not finish so there is still a bug somewhere and I cannot find it. I know that it's not calling the second destructor.

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

    copyPtr = listObj.head;

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

[ne555]

calm down, analyse it, write some pseudocode, make sure it is correct, then code.

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* nodePtr = nullptr;
    ListNode* newNode = new ListNode;
    ListNode* copyPtr = listObj.head;

    newNode->value = copyPtr->value; //if listObj is empty, you are dereferencing a null pointer
    newNode->next = nullptr;

    if(!head) //head is unititialized
        head = listObj.head; //shallow copy

    while(copyPtr->next != nullptr) //if listObj is empty, you are dereferencing a null pointer
    {
        appendNode(copyPtr->value); //don't know what this does
        copyPtr = copyPtr->next;
    }
}

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* newNode = nullptr;
    ListNode* copyPtr = listObj.head;

    newNode = new ListNode;
    head = newNode;
    newNode->value = copyPtr->value; //same problem with empty list
    copyPtr = copyPtr->next;

    while(copyPtr->next != nullptr) //now problem with list with just one element
    {
        newNode = new ListNode; //leaking memory
        newNode->value = copyPtr->value;
        copyPtr = copyPtr->next;
        //you never link your nodes
    }
}

LinkedList::LinkedList(const LinkedList& listObj)
{
    ListNode* newNode = nullptr; // Create new node /*your comment does not match with the code*/
    ListNode* copyPtr = nullptr; // Point to original object /*same here, ¿original object, what's that?*/

    copyPtr = listObj.head;

    while(copyPtr->next != nullptr) //same problem with empty list
    {
        newNode->next = new ListNode; //newNode is null in the first iteration, UB
        newNode->value = copyPtr->value;
    }
    //in the last node `next' holds garbage
}