I implemented LikedList in Java, everything working fine except deleteLast() method. I am relatively new, so failed debugging the issue despite being so trivial a problem

This is my implementation code in Java Where did I make the mistake?

package linkedlist;

public class LinkedList {
    private static Node first;
    private static Node last;

    private static class Node {
        private int value;
        private Node next;


        public Node(int val, Node nxt) {
            this.value = val;
            this.next = nxt;
        }

        public Node(int val) {
            this.value = val;
        }


        public static void main(String[] args) {
            addFirst(7);
            addFirst(6);
            addFirst(5);
            addFirst(4);
            addFirst(3);
            addFirst(2);
            addFirst(1);
            System.out.println("-----First And Last Node-----");
            System.out.println(first.value);
            System.out.println(last.value);
            deleteFirst();
            System.out.println("----After Deleting 1st Node------");
            System.out.println(first.value);
            System.out.println(last.value);
            deleteLast();
            System.out.println("-----After Deleting Last Node-----");
            System.out.println(first.value);
            System.out.println(last.value);
            deleteLast();
            System.out.println("-----After Deleting Last Node-----");
            System.out.println(first.value);
            System.out.println(last.value);
        }

        public static void addFirst(int val) {
            if (first == null) {
                first = last = new Node(val, null);
            } else {
                first = new Node(val, first);
            }
        }
        public static void addLast(int val) {
            if (first == null) {
                first = last = new Node(val, null);
            } else {
                last.next = new Node(val, null);
                last = last.next;
            }
        }

        public static void deleteFirst() {
            Node temp = first.next;
            first.next = null;
            first = temp;
            temp.next = null;
        }

        public static void deleteLast() {
            Node current = first;
            Node newLast = first;
            while (current != null) {
                if(current == last) break;
                newLast = current;
                current = current.next;
            }
            newLast.next = null;
            last = newLast;

        }

        public static int indexOf(int val) {
            Node current = first;
            int index = 0;
            while (current.value != val) {
                current = current.next;
                index  ;
            }
            return index;
        }
    }
}

---

The way the code is, whenever I run it the 1st and last node becomes of the same value, like for the above example code, after deleteLast() method 1st and last both nodes become 2 or having the value 2, which essentially means they are the same exact nodes

Output

-----First And Last Node-----
1
7
----After Deleting 1st Node------
2
7
-----After Deleting Last Node-----
2
2
-----After Deleting Last Node-----
2
2

CodePudding user response：

The problem is in deleteFirst: it always truncates the list to be at most one node.

The final statement temp.next = null should be removed, as it is identical to doing first.next = null. This is the correction:

    public static void deleteFirst() {
        Node temp = first.next;
        first.next = null;
        first = temp;
    }

But there should be no need to set the next member to null of the node that is being removed. It could just be:

    public static void deleteFirst() {
        first = first.next;
    }

CodePudding user response：

While I'm not sure if you've kept track of last properly, your logic in deleteLast() is unnecessarily complicated. You could easily just loop until curr.next.next is null then set curr.next to null and set last to curr.

Something like this:

public static void deleteLast() {
    node curr = first;
    while (curr.next.next != null) {
        curr = curr.next;
    }
    curr.next = null;
    last = curr;
}

Since you haven't shared exactly what the error is, there could be other reasons why your code doesn't work.

CodePudding user response：

@trincot has the correct answer - in my testing it seemed helpful to add a "walk the list" to uncover issues which you had. So here's that method:

public static void walkList() {
    Node n = first;
    System.out.print("Walk: ");
    while (n != null) {
        System.out.print(n.value " ");
        n = n.next;
    }
    System.out.println();
}

Adding a call to walkList after each operation yields a "before fix" and "after fix":

Before Fix

-----First And Last Node-----
1
7
Walk: 1 2 3 4 5 6 7 
----After Deleting 1st Node------
2
7
Walk: 2 
-----After Deleting Last Node-----
2
2
Walk: 2 
-----After Deleting Last Node-----
2
2
Walk: 2 

After Fix

-----First And Last Node-----
1
7
Walk: 1 2 3 4 5 6 7 
----After Deleting 1st Node------
2
7
Walk: 2 3 4 5 6 7 
-----After Deleting Last Node-----
2
6
Walk: 2 3 4 5 6 
-----After Deleting Last Node-----
2
5
Walk: 2 3 4 5