Ndtyjky

Node is not getting inserted at position. I am new to coding and working on linked list.here head is pointing to start which i have declared globally.please help me to modify the code

void insert_at_position(struct node **head, int x, int pos)

{

    struct node *temp,*t,*tails = *head;

    t = (struct node*)malloc(sizeof(struct node));

    if (start == NULL)

    {

        start = t;

        start->data = x;

        start->next = NULL;

        return;

    }

    count++;

    while (temp->next!=NULL)

    {

        tails = temp;

        temp = temp->next;

        if (i == pos)

        {

            tails->next=t;

            t->data=x;

            t->next=temp->next;

            return;

        }

        i++;

    }

}

1. What's the use of count variable and why is i not declared anywhere? Please correct it.




2. Inside your while condition you need to handle pos == 0 case-




3. Below code will work

int insertNode(int pos, int data)

{

    struct node *temp = NULL, *tails = NULL, *p = NULL;

    p = (struct node*)malloc(sizeof(struct node));

    p->data = data;

    p->next = NULL;

    if(pos == 0)

    {

        p->next = start;

        start = p;

        return 0;

    }

    temp = tails = start;



    while(temp != NULL)

    {

        temp = temp->next;

        pos--;



        if(pos == 0)

        {

            tails->next = p;

            p->next = temp;

            return 0;

        }

        tails = tails->next;

    }



    if(pos)

        printf("nError!!! Invalid position, can't insertn");

    return 0;

}

1. The one problem in your code is that the temp is not initialized so the temp->next will not point to next node do temp = *head during declaration.


2. i is not declared.


3. If start is NULL you are inserting new node to pos=0 position regardless of position given as parameter(pos) to the function.


4. In your code a new node will not be added to pos=0 when number of nodes in linked list is 1(only head node).


5. In your code the new node will only be inserted to pos+1 position and not pos. During while loop iteration when i == pos condition is met tails points to the node at position 2 so you should have been inserting the node between tails_pervious node and tails node but you are inserting it between tails and tails_next which is incorrect.

The below code can be used. The code has only few modifications from the original code(so that you can understant your mistakes easily).

void insert_at_position(struct node **head, int x, int pos)

{

  struct node *temp = *head,*t,*tails = *head;  //temp was not initialized in original code so temp->next will not point to next node.

  int i = 0; // i was not initialized in original code

  t=(struct node*)malloc(sizeof(struct node));

  if(t == NULL) // malloc fail case check

  {

      return;

  }

  if (pos == 0) // 0th position insertion handling section

  {

      t->data = x;

      t->next = start;

      start = t;

      return;

  }

  if(start == NULL) // It is better to not inset any node if  position != 0 and Start == NULL

  {

      free(t);  // If the node is not inserted free the allocated memory.

      return;

  }

  count++;    // Assuming count is an global variable

  while(temp != NULL)

  {

      tails = temp;    

      temp = temp->next;

      if(i+1 == pos) // In this case tails points to the previous location

      {

          tails->next=t;

          t->data=x;

          t->next=temp; // temp now points to the node that was originally at 'pos' position

          return;

      }

      i++;          

  }

  free(t);  // If the node is not inserted free the allocated memory.

}

But I would like to suggest some improvements:-

• It is better to use a return type for the function so that the caller function can check if the operation is successful.


• It is always a good programming practice to reduce the exit points in a function i.e try to reduce the return statements inside a function. You can easily negotiate the use of return statement with the use of few if-else checks(it is totaly worth it)

• dont allocate before you need it


• use the power of the pointer-to pointer


• handle the case of inserting at the top (pos =0)


• handle the case where pos > length of the list

void insert_at_position(struct node **head, int x, int pos)

{

    struct node *new;



    if (!head) { // defensive

          return;

          }



        // Walk the list, until we

        // reach the end

        // or we have passed 'pos' nodes

    for (       ; *head; head = &(*head)->next) {

        if (!pos) break;

        pos --;

        }



     if (pos) { // list too short

        return;

        }



    new = malloc (sizeof *new);

     if (!new) { // malloc failed

        return;

        }

    new->data = x;

    new->next = *head;

    *head = new;

}