1

I am writing a code which will store elements in linked list, however when i'm adding an element at the end of the list the output is always like this...

enter image description here i've already search from many tutorials and copying their code to see if it works on mine but it doesn't..


#include <iostream>
#define tab '\t'

using std::endl;

struct node {
    int data;
    node* next;
};

class Linkedlist {
private:
    node* head = new node;

public:
    Linkedlist(){
        head->next = NULL;
    }

    void addStart(int val){
        node* temp = new node;
        temp->data = val;
        temp->next = head;
        head = temp;
    } 
    
    void addEnd(int val){
        node* temp = new node;
        temp->data = val;
        temp->next = NULL;

        node* curr = new node;
        curr = head;
        while(curr->next != NULL){
            curr = curr->next;
        }
        curr->next = temp;
    } 

    void display(){
        if (head == NULL){
            std::cout << endl << "Linked List is EMPTY.";
        } else {
            int i = 1;
            node* curr = new node;
            curr = head;

            std::cout << endl << "The linked list contains..." << endl;
            while(curr->next != NULL){
                std::cout << "Elements " << i << ':' 
                    << tab << curr->data << endl;
                curr = curr->next;
                i++;
            }
        }
    }
};

The Output should be.. Element 1: 1 Element 2: 2 Element 3: 3 Element 4: 4

Jesay
  • 11
  • 1
  • `addEnd` never modifies `head`. A default (i.e. empty) list has a `head` that points to a `node` with an indeterminate `data` and a null `next`. You would have noticed this if you hadn't been in such a hurry to test the adding of multiple elements. (Judging by your creation of a new node in `display`, and *two* nodes in `addEnd`, you haven't quite grasped the point of `new`.) – molbdnilo Jul 04 '23 at 13:40
  • 1
    Note that without proper calls to `delete` this object leaks memory. – Chipster Jul 04 '23 at 14:04
  • Note: `head` and `next` do the exact same thing. The only difference between them is the name. If you can abstract away the name, you can save a lot of code, and code you don't have to write has no bugs. [Here is an example of how simple inserting an item into a list could be.](https://stackoverflow.com/a/59779376/4581301) – user4581301 Jul 04 '23 at 18:51
  • I've noticed that Stack Overflow users often have questions with linked lists. To close this issue a bit, invite everyone to explore the following link: Implementation of a singly linked list in C++ - https://pvs-studio.com/en/blog/terms/6684/ – AndreyKarpov Jul 05 '23 at 06:47

2 Answers2

2

If you use addEnd on an empty list, head will point to NULL. Then, when you do curr = head, curr is also NULL. Then, you do while (curr->next != NULL), which is illegal since curr is NULL, so curr->next points to unknown memory.

You can add a condition checking if head is NULL:

void addEnd(int val) {
    if (head == NULL) {
        this->addStart(val);
        return;
    }
    //the rest of the code
}

The above code checks if head is NULL. If it is, the list is empty, and adding an element to the end of the list is functionally equivalent to adding an element to its start. You can just use addStart in that case.

azhen7
  • 371
  • 1
  • 5
1

Bugs:

  1. at addStart() function:

temp->next = head; head = temp;

Problem: This code never kept the last nodes' next as NULL Solution: a) declared head as null b) writing code for 2 different conditions (when the head is NULL and not NULL):

 Linkedlist(){
        head= NULL;
    }

    void addStart(int val){
        node* temp = new node;
        temp->data = val;
        
        if(head == NULL){
            head = temp;
            head->next = NULL;
            return;
        }  else {
            temp->next = head;
            head = temp;
            return;
        }

           }
  1. You didn't display the last node. Soln: change while loop condition in display function from

curr->next != NULL

to

curr != NULL.

OVERALL CODE:

  #include <iostream>
    #define tab '\t'
    
    using std::endl;
    
    struct node {
        int data;
        node* next;
    };
    
    class Linkedlist {
    private:
        node* head = new node ;
        
       
    
    public:
        Linkedlist(){
            head= NULL;
        }
    
        void addStart(int val){
            node* temp = new node;
            temp->data = val;
            
            if(head == NULL){
                head = temp;
                head->next = NULL;
                return;
            }  else {
                temp->next = head;
                head = temp;
                return;
            }
    
               } 
        
        void addEnd(int val){
            node* temp = new node;
            temp->data = val;
            temp->next = NULL;
    
           node* curr = new node;
            curr = head;
    
           
            while(curr->next != NULL){
                curr = curr->next;
             
            }
            curr->next = temp;
           
        } 
    
        void display(){
            if (head == NULL){
                std::cout << endl << "Linked List is EMPTY.";
            } else {
                int i = 1;
                node* curr = new node;
                curr = head;
    
                std::cout << endl << "The linked list contains..." << endl;
                while(curr != NULL){
                    std::cout << "Elements " << i << ':' 
                        << tab << curr->data << endl;
                    curr = curr->next;
                    i++;
                }
            }
        }
    };
  • Isn't the `if(head == NULL)` condition unnecessary? With the original code, if `head` was `NULL`, `temp->next = head` would set the `next` property to `NULL`, and `head = temp` would set `head` to the new pointer. Setting `head = temp` and then `head->next = NULL` is the same behavior. – azhen7 Jul 04 '23 at 14:44
  • yeah, you can achieve the result by removing the condition also. Thanks for your suggestion. – Yunesh Shrestha Jul 04 '23 at 16:12