Create a new linked list by inserting Nodes

Question

I am new into coding and I am trying to create a linked list by inserting nodes. I have taken a class Node that defines the structure of the node. I think my code is fine and there are no errors or warnings on compiling, but there is some runtime error. Please check and help.

#include <iostream>

using namespace std;

class Node 
{
public:
   
   int data ;
   Node *next;
   
   Node *head = NULL;
   
   void Display (Node *p){
       while(p!=NULL)
       {
           cout<< p->data << " ";
           p=p->next;
           
       }
   }
   
   void InsertAtLast(Node *p , int index , int x)
   {
       Node *t;
       int i;
       
       if(index < 0 )
           return;
           
       t = new Node ;
       
       t->data = x;
       
       if(index == 0)
       {
           t->next = p ;
           head = t;
       }
       else{
           
           for(i=0 ; i<index-1 ; i++)
               p=p->next;
           t->next = p->next;
           p->next = t;
       }
   }
};


int main()
{
   Node *head = NULL;
   Node obj;
   obj.InsertAtLast(head , 0 , 150);
   obj.InsertAtLast(head , 3 , 200);
   
   obj.Display(head);
   
   return 0;
}

My best advice I can give is: Start your program in the debugger, step through the code line by line, and check if al variable values change as intended. Alternatively put some `cout` statements at crucial points of your solution, you think they must be executed. — πάντα ῥεῖ, Sep 10 '20 at 10:58
your program is compilable. If there is no warnings, nor errors, that mean it's right, bt after compile and run i got segmentation fault, and it's probably memory leak — Krzysztof Mochocki, Sep 10 '20 at 11:00
Check `for(i=0;inext;` and `obj.InsertAtLast(head, 3, 200);`. nullptr occurs — Johnny, Sep 10 '20 at 11:01
@KrzysztofMochocki That's not entierly true. A program can run fine w/o observing a SEGFAULT, but having logical flaws that hinder it to produce the desired output. Also memory leaks have no influence on behavior, besides well, the programm consumes memory unnecessary as long it runs. — πάντα ῥεῖ, Sep 10 '20 at 11:04
“InsertAtLast” is a peculiar name for a function that can insert at any position. — molbdnilo, Sep 10 '20 at 11:07
Your design is wrong because you are confusing a node with a list. A list has a head field which points to the first node, a node has a next field which points to the next node. Your class has both. You should sort out the design issue first, everything will be clearer once the design is correct. Split your Node class into two classes, one called List and one called Node. — john, Sep 10 '20 at 11:53
`I think my code is fine` ... `but there is a runtime error`. Therefore, unfortunately, your code is not fine. Most bugs are not compiler errors, they are runtime errors. — john, Sep 10 '20 at 11:55
@john Won't it be more confusing with two classes ?...Can you show how do i do that, because I am just new to this stuff ! ...and i know that my code is not fine , but nothing seems wrong to me inside the code , i am asking help, the reason I have shared my code :) — Kanika Mahajan, Sep 10 '20 at 17:19
Random link, https://www.codesdope.com/blog/article/c-linked-lists-in-c-singly-linked-list/, No idea of the quality but it does show how to implement a linked list using two classes. — john, Sep 10 '20 at 17:25

score 0 · Answer 1 · answered Sep 10 '20 at 11:12

0

This is not the way i would recommend to implement a linked list. My adjustments:

have only two fields in the class Node, data and next_node.
Always initiate next_node to NULL.
In order to insert to the last Node just iterate until your next_node is null.
In order to insert to an index k, iterate k-1 times and update the current node and your new node next_node field accordingly.

Goodluck (:

answered Sep 10 '20 at 11:12

Aviv Nachman

49
2

_In order to insert to the last Node just iterate until your next_node is null_: this approach works, but it is a very bad idea. Think about what happens if the list contains many nodes. Hint: google "shlemiel the painter's algorithm" – Jabberwocky Sep 10 '20 at 12:09

score 0 · Answer 2 · answered Sep 10 '20 at 12:43

Not the best solution but I made your code functional. Take a look below. There are few changes I made and are commented in the code.

#include <iostream>

using namespace std;

class Node
{
        public:
                int data ;
                Node *next;

                Node *head = NULL;

                void Display(){
                        Node *it = head;
                        while(it!=NULL)
                        {
                                cout<< it->data << " ";
                                it=it->next;
                        }
                }

                void InsertAtIndex(int index , int x) /*Name of the function should be InsertAtIndex instead of InsertAtLast */
                {
                        if(index < 0 )
                                return;

                        if(index == 0)
                        {
                                if(head == NULL){ /*if the list is empty*/
                                        head = new Node;
                                        head->data = x;
                                }
                                else {  /*insert at head when the list is not empty*/
                                        Node *tmpHead = head;
                                        Node *node = new Node;
                                        node->data = x;
                                        head = node;
                                        node->next = tmpHead;
                                }
                        }
                        else{
                                Node *node = head;
                                Node *tmp = NULL;
                                int i = 1;
                                while(node){
                                        if(i == index) {
                                                /*create a new node*/
                                                tmp = new Node;
                                                tmp->data = x;

                                                /*backup next node, if it exists*/
                                                Node *nextNode = node->next;

                                                /*insert the node*/
                                                node->next = tmp;
                                                tmp->next = nextNode;
                                                break;
                                        }
                                        node = node->next;
                                        i++;
                                }
                                if(tmp == NULL)
                                        std::cout << "insertion failed at index: " << index << " because a node doesn't exist at index: "<< index - 1 << std::endl;
                        }
                }
};


int main()
{
        Node obj;
        obj.InsertAtIndex(0 , 150);
        obj.InsertAtIndex(1 , 151);
        obj.InsertAtIndex(2 , 152);
        obj.InsertAtIndex(3 , 200);

        obj.InsertAtIndex(3 , 201);//Inserting at index 3 again

        obj.InsertAtIndex(0 , 100);//Inserting at index 0 again

        obj.Display();

        return 0;
}

Below is the output:

rohit@linux:~/cpp$ ./list
100 150 151 152 201 200 rohit@linux:~/cpp$
rohit@linux:~/cpp$

Yeah , This can be one of my ways ... I have found one more way.If I pass double pointer(**p) instead of single pointer(*p) in the function , and modify the code accordingly , this would work .Since I am pointing to address of another pointer , I need double pointer! — Kanika Mahajan, Sep 10 '20 at 17:23

Create a new linked list by inserting Nodes

2 Answers2