Problem in implementing a linkedlist in c++

Question

I am learning C++ and trying to implement a singly linkedlist. Basically a linkedlist providing two simple methods, insert and display. Below is the code snippet --

class LinkedList {


public:
    int data;
    LinkedList *next;
    LinkedList() {

    }

    LinkedList(int d) {
        data = d;
        

    }

    void insert(int d) {
        LinkedList temp = *this;
        cout << "Calling insert "<< temp.data;
        while (temp.next != NULL) {
            cout << "Inside the while loop"<< temp.data;
            temp = *temp.next;

        }           
        temp.next = new LinkedList(d);      

        


    }

    void display() {
        LinkedList temp = *this;
        cout << temp.data;          
        

        while (&temp != NULL) {             
            cout << temp.data;    
            temp = *temp.next;
            cout << temp.data;              
        } 
        


    } 




};



int main()
{
    LinkedList head(1);
    head.insert(2);
    head.insert(3);
    head.insert(4);
    head.display();

    return 0;
}

However, this insert method is not working as expected. Even after calling insert multiple times, it is never going inside the while loop and "Inside the while loop" is never getting printed. Could you guide what is wrong with my insert method?

LinkedList temp = *this; makes a copy and then you modify the copy. — Retired Ninja
– Retired Ninja, Commented Jun 28, 2021 at 5:37
However, this insert method is not working as expected -- You failed to initialize all of your member variables in both of your constructors, and that is just one of a multitude of issues. int main() { LinkedList x; x.insert(10); } -- you don't need any further code to see the issues. — PaulMcKenzie
– PaulMcKenzie, Commented Jun 28, 2021 at 5:37
LinkedList temp = *this; creates a local copy of this. Hence, temp.next = new LinkedList(d); appends the new node to the local copy and is lost as soon as you leave insert(). — Scheff's Cat
– Scheff's Cat, Commented Jun 28, 2021 at 5:38

luantkow · Accepted Answer · 2021-06-28 06:08:01Z

You have to improve more than one thing to keep the code running.

Initialize the value of next in constructor. Otherwise you cannot be sure that when LinkedList is created next is set to NULL
```
LinkedList(int d) : data(d), next(NULL) {}
```
Use LinkedList* instead of LinkedList as a temp type. Otherwise you won't be modifying the List nodes but local copy of first node. Please note that:
- LinkedList temp = *this; will have to be replaced to LinkedList* temp = this;
- all occurrences of temp.next will have to be replaced to temp->next
- all occurrences of temp.data will have to be replace to temp->data
- the while condition in display function will have to replaced: &temp != NULL -> temp != NULL
The third line in a while loop in display function has to be removed as temp may point to NULL.

The minor thing is that you did not break lines in your output. Please make sure to add << endl in places where you want the line to break.

The big problem is that you still have to implement destructor for the LinkedList to release the allocated resources. According to rule of three, with a destructor you should also define copy constructor and assignment operator.

The other approach would be to use a smart pointer like std::unique_ptr to store the address of next node.

#include <iostream>
#include <memory>

using namespace std;

class LinkedList {
public:
    int data;
    std::unique_ptr<LinkedList> next;

    LinkedList(int d) : data(d) {}

    void insert(int d) {
        LinkedList* temp = this;
        cout << "Calling insert "<< d << std::endl;
        while (temp->next) {
            cout << "Inside the while loop: "<< temp->data << std::endl;
            temp = temp->next.get();

        }           
        temp->next = std::make_unique<LinkedList>(d);      
    }

    void display() {
        LinkedList* temp = this;
        
        while (temp != NULL) {             
            cout << temp->data << std::endl;
            temp = temp->next.get();
        } 
    } 
};

int main()
{
    LinkedList head(1);
    head.insert(2);
    head.insert(3);
    head.insert(4);
    head.display();

    return 0;
}

In C++11 and newer, nullptr should be used instead of NULL.

Collectives™ on Stack Overflow

Problem in implementing a linkedlist in c++

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest