NullPointerException LinkedList reverse method

Question

I'm stuck on this exercise where I have to implement a reverse method for a doubly linked list, which reverses the entire list. I'm getting a NullPointerException in my for loop and I'm not sure how to solve it as in the exercise it is stated that I am not supposed to create new IntNode instances. I keep getting the exception even if I use an if-else statement if(current.next != null) ... Any help would be much appreciated!

void reverse() {
        IntNode temp1;
        IntNode temp2;
        // TODO: Vervollständigen Sie die Methode wie in der Aufgabenstellung gefordert.
        
        this.last.next = this.last.prev;
        this.last.prev = null; 
        
        for (IntNode current = this.last.next; current != this.first; current = current.prev) {
                temp1 = current.next;

            current.next = current.prev;
            current.prev = temp1;
        }
        this.first.prev = this.first.next;
        this.first.next = null;
        
        temp2 = this.first;
        this.first = this.last;
        this.last = temp2;
    }

Does this answer your question? [What is a NullPointerException, and how do I fix it?](https://stackoverflow.com/questions/218384/what-is-a-nullpointerexception-and-how-do-i-fix-it). You should spend time debugging your program, going through line by line, or just reading the stack trace to identify the line causing the issue. — Henry Twist, Jun 30 '21 at 15:43
Hello helloworld123. Welcome to the Stacks. Have you walked through the code by hand with paper noting down latest values of variables for a simple case that cause the problem? — Alan, Jun 30 '21 at 15:47
Welcome to SO. Please provide a full executable example: the complete class and the execution data on which you're getting the Exception. Best regards. — Dmitriy Popov, Jun 30 '21 at 17:21

score 0 · Answer 1 · answered Jun 30 '21 at 16:04

As for each node you are swapping next and prev of current node therefore, the increment condition in for loop should be current = current.next instead of current = current.prev.

    void reverse() {
        IntNode temp1;
        IntNode temp2;
    
        this.last.next = this.last.prev;
        this.last.prev = null; 
    
        for (IntNode current = this.last.next; current != this.first; current = current.next) {
            temp1 = current.next;

            current.next = current.prev;
            current.prev = temp1;
        }
        this.first.prev = this.first.next;
        this.first.next = null;
    
        temp2 = this.first;
        this.first = this.last;
        this.last = temp2;
    }

csalmhof · Answer 2 · 2021-07-08T05:53:42.340

When swapping, the next element to swap becomes the next of the current inspected node. So you need to set current = current.next for the next iteration of the loop.

You also dont need to swap the last and first element outside of the loop, you can just do this inside, when you change the condition of the for-loop: Start with the last node and go until the inspected current is null.

Here is a slightly shorter solution based on your code:

public void reverse() {
  for (IntNode current = this.last; current != null; current = current.next) {
    IntNode temp1 = current.next;
    current.next = current.prev;
    current.prev = temp1;
  }
  
  IntNode temp2 = this.first;
  this.first = this.last;
  this.last = temp2;
}

NullPointerException LinkedList reverse method

2 Answers2