How do I fix that infinite loop on a custom double linked list?

Question

I have created a custom LinkedList and I am trying to have a method that partitions it based on a value. There is no need to maintain the order so I thought of adding values less than x to the head of the list and values equal or greater to the tail.

Nonetheless, there is an infinite loop happening as node.next is never null when the check is happening.

Here is the code:

public class LinkedList<T extends Comparable<T>> {
    private int size = 0;
    private Node<T> head = null;
    private Node<T> tail = null;

    public int size() {
        return size;
    }

    public boolean isEmpty() {
        return size == 0;
    }

    /**
     *
     * @param t the new element to be added at the end of the list
     *          NOTE: null items are not allowed
     *
     * @return true if added
     */
    public boolean add(T t) {

        if (t == null) {
            return false;
        }
        // if head is null add as head
        if (head == null) {
            head = new Node<>(t, null, null);
            tail = head;
        } else {
            Node<T> newTail = new Node<>(t, tail, null);
            tail.next = newTail;
            tail = newTail;
        }
        size++;
        return true;
    }

    void partition(T value) {
        Node<T> node = head;

        while (node != null) {
            Node<T> prev = node.previous;
            Node<T> next = node.next;

            if (node.data.compareTo(value) >= 1) {
                // should go to tail
                if (node != tail) {
                    prev.next = node.next;
                    next.previous = node.previous;

                    tail.next = node;
                    node.previous = tail;
                    node.next = null;
                    tail = node;
                }

            } else {
                // should go to head
                if (node != head) {
                    prev.next = node.next;
                    if (null != next) {
                        next.previous = node.previous;
                    }

                    head.previous = node;
                    node.next = head;
                    head = node;
                }
            }

            node = next;
        }

        head.previous = null;
        tail.next = null;
    }

    public T getFirst() {
        return head.data;
    }

    public Node<T> getHead() {
        return head;
    }

    public T getLast() {
        return tail.data;
    }


    private static class Node<T extends Comparable<T>>  {
        T data;
        Node<T> previous;
        Node<T> next;

        Node(T data, Node<T> previous, Node<T> next) {
            this.data = data;
            this.previous = previous;
            this.next = next;
        }
    }

}

Test case to reproduce:

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import java.util.Arrays;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;


public class LinkedListTest {

    private LinkedList<Integer> linkedList;

    @BeforeMethod
    public void setUp() {
        linkedList = new LinkedList<>();
    }

    @Test
    public void testPartition() {
        linkedList.add(0);
        linkedList.add(9);
        linkedList.add(2);
        linkedList.add(5);
        linkedList.add(4);
        linkedList.add(7);
        linkedList.add(1);

        int[] actual = new int[linkedList.size()];

        linkedList.partition(5);

        for (int i = 0; i < linkedList.size(); ++i) {
            actual[i] = linkedList.get(i);
        }

        System.out.println(Arrays.toString(actual));
    }

}

Answer 1

Let's say your list has two items: A->B and both A and B > value (so "should go to tail"). Begin your code. Node A > value, so A gets put on the tail. So now your list is B->A and B is the 'next' node and A is now the tail.

The next iteration of the loop looks at Node B. Node B > value, so B is put on the tail. Now your list is A->B and A is the 'next' node and B is now the tail. Obviously this is an infinite loop.

To solve this in the spirit of how this code is written, before starting the loop do this:

last_node = tail;

Now, just before you have node = next; do this:

if(node == last_node) break;

In the example above, last_node points to B. The loop processes A then B then quits because B is last_node.