Reputation: 1
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
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
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