Move last node of the linked list to the front Description:

Given a linked list move the last node to the front.

For example:


  10 20 30 40 -> 40 10 20 30


Code:

class Main {
  static class Node {
    private int data;
    private Node next;

    Node(int data, Node next) {
      this.data = data;
      this.next = next;
    }

    Node append(int data) {
      Node newNode = new Node(data, null);
      Node current = this;

      while (current.next != null) {
        current = current.next;
      }
      current.next = newNode;
      return this;
    }

    public String toString() {
      Node current = this;
      StringBuilder sb = new StringBuilder();

      while (current != null) {
        // do something here
        sb.append(current.data);
        sb.append(" ");
        current = current.next;
      }
      return sb.toString();
    }
  }

  public static Node moveTailToHead(Node head) {
    if (head == null || head.next == null) {
      throw new IllegalStateException(
          "List should have atleast two elements");
    }

    Node current = head;
    Node prev = null;

    while (current.next != null) {
      prev = current;
      current = current.next;
    }
    prev.next = null;
    current.next = head;
    head = current;

    return head;
  }

  public static void main(String[] args) {
    Node head = new Node(10, null);
    head.append(20)
        .append(30)
        .append(40);
    System.out.println(head); // 10 20 30 40

    Node newHead = moveTailToHead(head);
    System.out.println(newHead); // 40 10 20 30

    try {
      System.out.println(
        "Should not happen: " + moveTailToHead(null));
    } catch (IllegalStateException e) {
      System.out.println(
        "Got expected exception for null node");
    }

    try {
      System.out.println(
        "Should not happen: " + moveTailToHead(new Node(10, null)));
    } catch (IllegalStateException e) {
      System.out.println(
        "Got expected exception for single node");
    }
  }
}


Question:

I am writing code in Java after a long time and would like to know if I am making any mistake or abusing any feature of Java.
You are only able to access Node's fields data and next from the method moveTailToHead(Node) because this method is declared in the same top level class as Node. I don't know why you chose this design for your code sample, but the fact that you made the method moveTailToHead a static method outside the class Node rather than an instance method of Node makes me think that moveTailToHead is also supposed to work "from the outside" without having access to Node's internal implementation details, so I'm wondering whether your code design in this regard is intentional. If it is, then I won't argue against it, but if it isn't and if moveTailToHead should also work if it were declared in a different top level class, then you need to implement a way for Node to provide some way of accessing the contents of the list it represents. A way to start could be for Node to implement the interface Iterable&lt;Integer&gt;, which would require Node to have an iterator() method. This method could, for instance, look like this:

@Override
public Iterator&lt;Integer&gt; iterator() {
    return new Iterator&lt;Integer&gt;() {
        private Node nextNode = Node.this; //the next node to be returned by next()
        private Node currentNode = null;   //the last node returned by next()
        private Node previousNode = null;  //the node that precedes currentNode

        @Override
        public boolean hasNext() {
            return nextNode != null;
        }

        @Override
        public Integer next() {
            if (!hasNext()) {
                throw new NoSuchElementException();
            } else {
                if (currentNode != null) {
                    previousNode = currentNode;
                }
                currentNode = nextNode;
                int nextData = nextNode.data;
                nextNode = nextNode.next;
                return nextData;
            }
        }

        @Override
        public void remove() {
            if (currentNode == null) {
                throw new IllegalStateException();
            } else if (previousNode != null) {
                currentNode.next = null;
                previousNode.next = nextNode;
                currentNode = null;
            } else {
                assert currentNode == Node.this;
                throw new UnsupportedOperationException();
            }
        }
    };
}


That way, you can also get rid of the code duplication whenever you iterate through the list using constructs like while (current.next != null) or similar.

If you look at the method remove(), it becomes apparent that the fact that you are using a Node object to refer to the actual list itself, which Xtreme Biker already criticized in his answer, is problematic. Since the head node also defines the list, it is not possible to remove the head node from the list, which is why I made the method throw an UnsupportedOperationException for the sake of completeness in this demonstration code.

But apart from this issue, with Iterable implemented, adding other methods to add or remove elements should not be very difficult, and then you can move the tail node to the head without having direct access to the private fields Node.data and Node.next.