Search code examples
cdata-structureslinked-listdynamic-memory-allocation

Adding a node containing multi-digit number at the nth position in a linked list


I have written a code for inserting a node at nth position.
When a user inputs 1 digit number in a node then it works perfectly but when user inputs equal to or more than two digit numbers then its keep on printing the last node only in infinite loop.

I couldn't figure out what's went wrong. My code is below

#include<stdio.h>
#include<stdlib.h>

struct st
{
    int roll;
    char name[20];
    struct st *next;
};

void add_middle(struct st **ptr)
{
    struct st *temp,*temp1;
    temp=malloc(sizeof(struct st ));
    printf("netre ur name\n");
    scanf("%s",temp->name);
    printf("enter ur roll\n");
    scanf("%d",&(temp->roll));

    if((*ptr==NULL)||(temp->roll<(*ptr)->roll))
    {
        temp->next=*ptr;
        *ptr=temp;
    }
    else
    {
        temp1=*ptr;
        while(temp1)
        {
            if((temp1->next==NULL)||(temp1->next->roll>temp->roll))
            {
                temp1->next=temp;
                temp->next=temp1->next;
                break;
            }
            temp1=temp1->next;
        }

    }
}

void display(struct st *ptr)
{
    while(ptr)
    {
        printf("%s %d\n",ptr->name,ptr->roll);
        ptr=ptr->next;
    }
}

main()
{
    struct st *headptr=0;
    add_middle(&headptr);`
        add_middle(&headptr);
    add_middle(&headptr);
    display(headptr);
}

Solution

  • Let's look at what happens when you insert the new node:

    temp1->next = temp;
    temp->next = temp1->next;
    

    This will make the previous node (temp1) point to the new node, which is good. It will then have the new node (temp) point to itself (temp1->next == temp), which is bad.

    To fix this, you can just swap those two lines. That is:

    if ((temp1->next==NULL) || (temp1->next->roll > temp->roll)) {
        temp->next = temp1->next;
        temp1->next = temp;
        break;
    }
    

    Additionally, this could be much clearer if you used better variable names:

    • temp1 becomes previousNode
    • temp becomes newNode