CODEWITHSUNDEEP
c++data-structuressingly-linked-list
user496934
user496934

Reputation: 4020

Problem in implementing a linkedlist in c++

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?

Upvotes: 0

Views: 63

Answers (1)

luantkow
luantkow

Reputation: 2949

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

  1. 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) {}

  2. 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

  3. 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;
}

Upvotes: 1

Related Questions