TWZ00
TWZ00

Reputation: 1

Why is my C code running an infinite loop and how do I obtain my expected output?

Why is my C code running an infinite loop and how do I fix it to obtain my expected output ?

I am supposed to reverse nodes from A to B

So let's say I give an input of :

2 5

1 2 3 4 5 6 7

The first line is to indicate from which index to which index I am supposed to reverse the nodes So here 3 is node A and 6 is node B. The second line is the list I have given. So, I am expecting an output of : 1 2 6 5 4 3 7

This is my code :

ListNode *reverseSegment(ListNode *head, int start, int end)
{
  // Write your code here

  // Condition to Check if B is valid (if B is valid, then A is valid)
  ListNode *temp1 = head;
  int count = 0;
  while (temp1 != NULL)
  {
    temp1 = temp1->next;
    count++;
  }

  // Condtion to check if index out of range or invalid
  if (count < end)
  {
    return head;
  }

  // Declaring Pointers
  ListNode *prev = NULL, *cur = head, *next = NULL, *endptr = NULL;
  ListNode *temp2 = head; 
  int diff = end - start;

  // Point endptr to the last node of the list
  while (temp2->next != NULL)
  {
    endptr = temp2; 
  }
  // Point next ptr to the node after A
  // Point prev pointer to the node after B
  //next = cur->next;
  prev = endptr;
  
  // Reversing the node from B to A
  while ((diff+1) > 0 )
  {
    next = cur->next;
    cur->next = prev;
    prev = cur;
    cur = next;
    diff--;
  }
  // Point head ptr to node B
  head->next = prev;

  //return head;
} 

Upvotes: 0

Views: 127

Answers (2)

Aconcagua
Aconcagua

Reputation: 25546

You are doing stuff a bit too complex, especially you are iterating around too much.

At very first you might want to check, if start indeed is smaller than end as well! By the way: Negative indices are pretty meaningless, so I'd recommend unsigned types to specify them, e.g. size_t.

You are now interested in the offset between start and end, so I'd calculate that one first:

end -= start;

You won't need the original value any more, see below.

Then you iterate from head onward:

if(!head || start == end) // empty list or sequence
{
    return head;
}

ListNode* startNode = head;
for(;start; --start)
{
    if(!startNode->next)
    {
       // no nodes left -> start is not valid!
       return head;
    }
    startNode = startNode->next;
}
ListNode* endNode = startNode;
for(;end; --end)
{
    if(!endNode->next)
    {
       // no nodes left -> end is not valid!
       return head;
    }
    endNode = endNode->next;
}

Now both start and end are down-counted to 0, but you don't need them any more anyway – you found both start and end node, and from now on you can operate on the nodes directly.

Within the list segment you now can reverse by simply swapping the prev and next pointers for every node:

for(ListNode* cur = endNode; cur != startNode;  cur = cur->next)
// note that you swapped pointers already when
// getting here so you still iterate backwards: ^^^^^^^^^^^^^^^
{
    ListNode* tmp = cur->next;
    cur->next = cur->prev;
    cur->prev = tmp;
}

Now left to fix: endNode's predecessor points to what has been its successor, which, though, needs to get the successor of the start node, whereas the start node hasn't been changed at all yet. So we need:

ListNode* tmp = endNode->prev;
endNode->prev = startNode->prev;   // tail now fixed completely
startNode->prev = startNode->next; // swap direction
startNode->next = tmp;             // set to the remaining list

Finally:

return endNode->prev ? head : endNode;

If startNode has a predecessor then we reversed from somewhere in the middle onward, thus need to return head – otherwise we reversed from the beginning of the list and original head node has been moved away, while the original end node now is the new head.

Be aware that above is untested code – if you discover a bug, please fix yourself...

Upvotes: 0

Jonathon S.
Jonathon S.

Reputation: 1980

This while loop will be infinite since temp2 is never modified in each loop:

  // Point endptr to the last node of the list
  while (temp2->next != NULL)
  {
    endptr = temp2; 
  }

It looks like you really need to search for the node before A and B, so something like this may be simpler solution (for start > 0):

  int      count = 0;
  ListNode *cur = head;

  ListNode *start_node_prev;
  ListNode *start_node;
  ListNode *start_node_next;
  ListNode *end_node_prev;
  ListNode *end_node;
  ListNode *end_node_next;

  while(count < start)
  {
     start_node_prev = cur;
     cur             = cur->next;
     start_node      = cur;
     start_node_next = cur->next;
  }

  while(count < end)
  {
     end_node_prev = cur;
     cur           = cur->next;
     end_node      = cur;
     end_node_next = cur->next;
  }

  start_node->next = end_node_next;
  end_node->next   = start_node_next;

  start_node_prev->next = end_node;
  end_node_prev->next   = start_node;

Note that a separate case is needed if start == 0.

Also, a check should be include for start < end.

Upvotes: 0

Related Questions