Reverse doubly-link list in C++

Question

I've been trying to figure out how to reverse the order of a doubly-linked list, but for some reason, in my function void reverse() runs while loop once and then crashes for some reason. To answer some questions ahead, I'm self-teaching myself with my brothers help. This isn't all of the code, but I have a display() function which prints all nodes chronologically from start_ptr and a switch which activates certain functions like

    case 1 : add_end(); break;
    case 2 : add_begin(); break;
    case 3 : add_index(); break;
    case 4 : del_end(); break;
    case 5 : del_begin(); break;
    case 6 : reverse(); break;

This is the geist of my code:

#include <iostream>
using namespace std;

struct node
{
    char name[20];
    char profession[20];
    int age;
    node *nxt;
    node *prv;
};

node *start_ptr = NULL;

void pswap (node *pa, node *pb)
{
    node temp = *pa;
    *pa = *pb;
    *pb = temp;
    return;
}

void reverse()
{
    if(start_ptr==NULL)
    {
        cout << "Can't do anything" << endl;
    }
    else if(start_ptr->nxt==NULL)
    {
        return;
    }
    else
    {
        node *current = start_ptr;
        node *nextone = start_ptr;
        nextone=nextone->nxt->nxt;
        current=current->nxt;
        start_ptr->prv=start_ptr->nxt;
        start_ptr->nxt=NULL;
        //nextone=nextone->nxt;
        while(nextone->nxt!= NULL)
        {
            pswap(current->nxt, current->prv);
            current=nextone;
            nextone=nextone->nxt;
        }
        start_ptr=nextone;
    }
}

Answer 1

Try this:

node *ptr = start_ptr;
while (ptr != NULL) {
    node *tmp = ptr->nxt;
    ptr->nxt = ptr->prv;
    ptr->prv = tmp;
    if (tmp == NULL) {
        end_ptr = start_ptr;
        start_ptr = ptr;
    }
    ptr = tmp;
}

Answer 2

You can simplify your reverse() quite a bit. I'd do something like this:

void reverse()
{
    if(start_ptr == NULL)
    {
        cout << "Can't do anything" << endl;
    }
    else
    {
        node *curr = start_ptr;
        while(curr != NULL)
        {
            Node *next = curr->next;
            curr->next = curr->prev;
            curr->prev = next;
            curr = next;
        }
        start_ptr = prev;       
    }
}

Explanation: The basic idea is simply to visit each Node and swap the links to previous and next. When we move curr to the next Node, we need to store the next node so we still have a pointer to it when we set curr.next to prev.

Answer 3

EDIT: My first implementation, which was correct but not perfect. Your implementation is pretty complicated. Can you try this instead:

node * reverse(Node * start_ptr)
{
    Node *curr = start_ptr; 
    Node * prev = null;
    Node * next = null;
    while(curr)
    {
        next = curr->nxt;
        curr->nxt = prev;
    curr->prv = next;
        prev = curr;
        curr = next;
    }
    return start_ptr=prev;
}

Here is my updated solution:

node * reverse()
{
    node *curr = start_ptr; 
    node * prev = NULL;
    node * next = NULL;
    while(curr)
    {
        next = curr->nxt;
        curr->nxt = prev;
        curr->prv = next;
        prev = curr;
        curr = next;
    }
    return start_ptr=prev;
}

The logic was correct. But the issue was that I was accepting in input argument start_ptr. Which means that I was returning the local copy of it. Now it should be working.

Answer 4

Your pswap function is wrong your should swap the pointer not try to create temporary objects and swap them. Should be like that (there might be other mistake later)

void pswap (node *&pa, node *&pb)
{
    node* temp = pa;
    pa = pb;
    pb = temp;
    return;
}

Answer 5

I suggest maintaining a link to the last node.
If not, find the last node. Traverse the list using the "previous" links (or in your case, prv).

There is no need to actually change the links around. Traversing using the prv pointer will automatically visit the nodes in reverse order.

Answer 6

Look at

valuesnextone=nextone->nxt->nxt;

Here nextone->nxt can be null.

Apart from that, try to use pointers to pointers in the swap function.