cpd1
cpd1

Reputation: 789

Having a little trouble with pointers

The below is meant to reverse a linked list. It seems to work till it gets to the last line. When I debug, both "current" and "result" are of the same type (Node*) and "result" is the list reversed. But when the function completes, current only has the first value of the "result" list. Anyone know why "current" is not the full list when the function completes?

struct Node {
   int data;
   Node* next;
};

void reverseList(Node** head)
{
    Node* current = *head;
    Node* result = NULL;

    while (current != NULL)
    {
        Node* temp = current;
        current = temp->next;
        temp->next = result;
        result = temp;
    }

    current = result;
}

Upvotes: 0

Views: 58

Answers (2)

Sam Varshavchik
Sam Varshavchik

Reputation: 118435

There are multiple problems with the shown logic.

We can start with the obvious observation that the goal of reverseList is, apparently, to reverse a singly-linked list.

The second observation is that the function takes a single parameter, a pointer to the head node, and it returns a void.

From, that we conclude that the function should update the head node, but there's nothing in the code that does that.

Additionally, there's really no reason why this function should take a double pointer like that, a pointer to the head node, and update it. It's much simpler for the function to take an ordinary pointer to the first element of the list, the existing head pointer, and then return the head pointer of the reversed list.

With this simple change, the resulting logic becomes much, much shorter and simpler:

Node *reverseList(Node *head)
{
    Node *current=NULL;

    while (head)
    {
        Node *next=head->next;

        head->next=current;
        current=head;

        head=next;
    }
    return current;
}

That's it.

Upvotes: 3

Franck
Franck

Reputation: 1635

you need to update the head at the end of your algorithm:

current = result;
*head = current;

Upvotes: 2

Related Questions