-1

I am new at linked List. Recently I have tried to create a program which takes an array and its size as input. then it converts the array into a linked list and print elements. But the program is not working and I guess it is because the head pointer get changed. So, what I can do to keep the head node unchanged?

#include<bits/stdc++.h>
using namespace std ;
struct node
{
    int data ;
    node* link ;
};

node* create_linkedlist (int ara[] , int siz )
{
    node* head = NULL ;
    node* temp = new node() ;
    temp->data = ara[0] ;
    temp->link = NULL ;
    head = temp ;
    node* tmhead = head->link ;
    node* temp2 ;
    for(int i = 1 ; i < siz ; i++)
    {
        temp2 = new node() ;
        temp2->data = ara[i] ;
        temp2->link = NULL ;

        while ( tmhead->link!= NULL)
        {
            tmhead = tmhead->link ;
        }

        tmhead->link = temp2 ;
    }

    return head ;
}

void printlist( node* h_ref )
{
    while (h_ref != NULL)
    {
        printf("%d " , h_ref->data) ;
         h_ref = h_ref->link ;
    }
}


int main()
{
    int siz ;
    cin>> siz ;
    int ara[siz + 2];
    for(int i = 0  ; i < siz ; i++)
    {
        cin >> ara[i] ;
    }
    node* hd = create_linkedlist(ara , siz) ;
    node* temp = hd ;
    printlist(temp) ;
    return 0 ;
}
Nicol Bolas
  • 378,677
  • 53
  • 635
  • 829

1 Answers1

0

For the second element (first iteration of loop) in create_linkedlist() function, tmhead is NULL and you are trying to de-reference it which will lead to a crash.

Change the line node* tmhead = head->link ; to node* tmhead = head; and it should work fine.

Also try to use specific header instead of bits/stdc++.h and use nullptr instead of NULL. You don't need the while loop inside the for loop as well. Get rid of it and just change the for loop like below -

for(int i = 1 ; i < siz ; i++)
{
    temp2 = new node() ;
    temp2->data = ara[i] ;
    temp2->link = NULL ;

    tmhead->link = temp2 ;
    tmhead = tmhead->link ;
}

Notice that in case of user provided size 0, your code acts wrongly. Better to include header node creation, population inside the loop as well.

With all the mentioned changes the function may look like this -

node* create_linkedlist (int ara[] , int siz )
{
    node* head = nullptr;
    node* temp = nullptr;
    node* last = nullptr; // Keeps track of last inserted node
    for(int i = 0; i < siz; i++)
    {
        temp = new node();
        temp->data = ara[i];
        temp->link = nullptr;

        if (!head) {
          head = temp;
        }

        if (last) {
          last->link = temp;
        }
        last = temp;
    }
    return head ;
}
kuro
  • 2,585
  • 1
  • 12
  • 23
  • if (last) { last->link = temp; } last = temp what did you mean by it? And why I have to use specific header file name instead of bits/stdc++.h? – MD.Maruf Hasam Nov 23 '19 at 05:14
  • @MD.MarufHasam, `if (last)` checks whether `last` holds a valid pointer instead of `nullptr`. If it is valid then we will de-reference it and add the newly created node at end of the list. So, in first iteration, this `if` block will not be executed. But to keep track of the last inserted node, we will assign `last` to newly created node always (even in the first iteration). For the second question, please refer to - https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h – kuro Nov 23 '19 at 05:30