Delete a node from a linkedlist

Question

class StackNode{
    int data;
    StackNode next;

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

public class StackWithLinkedList {

    StackNode root = null;

    public void push(int data){
        if(root == null)
            root = new StackNode(data, null);
        else {
            StackNode temp = root;
            while(temp.next != null)
                temp = temp.next;

            temp.next = new StackNode(data, null);
        }
    }

    public int pop() throws Exception{
        if(root == null)
            throw new Exception("No Elements in Stack");
        else {
            StackNode temp = root;
            while(temp.next != null)
                temp = temp.next;
            int data = temp.data;
            temp = null;
            return data;
        }
    }


    public void print(){
        StackNode temp = root;
        while(temp!= null){
            System.out.print(temp.data +" ");
            temp = temp.next;
        }
        System.out.print("\n");
    }

    public static void main(String[] args) {
        StackWithLinkedList stack = new StackWithLinkedList();
        for(int i = 1; i<=15; i++){
            Random randomGen = new Random();
            stack.push(randomGen.nextInt(i));
        }
        stack.print();
        System.out.print("\n");
        try {
            System.out.println("Deleted: "+stack.pop());
            System.out.println("Deleted: "+stack.pop());
        } catch (Exception e) {
            e.printStackTrace();
        }
        stack.print();
    }
}

I am trying to implement Stack with Linkedlist. In pop function i am traversing till the last node and making it null. When i print the list. it remain unchanged. Does assigning root to temp and traversing with it cause any problem?

Answer 1

public int pop() throws Exception{
   if(root == null)
       throw new Exception("No Elements in Stack");
   else {
       StackNode temp = root;
       StackNode prev;
       while(temp.next != null){
           prev = temp;
           temp = temp.next;
       }
       int data = temp.data;
       prev.next = null;
       return data;
   }

}

Answer 2

You can avoid this all together by simplifying your implementation. It's just a stack so it's LIFO. All you need to do is make the last element push-ed the root. When pop-ing return the data in root and set root to the next in line.

The way you're doing it increases the typical complexity of O(1) to O(N) for the push and pop operations.

Something like:

public void push(int data) {
    root = new StackNode(data, root);
}

public int pop() throws Exception {
    if (root == null)
        throw new Exception("No Elements in Stack");
    else {
        int data = root.data;
        root = root.next;
        return data;
    }
}

Answer 3

As mentioned in other answer by @ChiefTwoPencils, that must be the preferred way of implementing this. However, to correct your logic of pop operation you shall keep track of second last item and once you get that you can return data value of next node and set the next link to null.

Here is changed logic from your code of pop method

     public int pop() throws Exception{
        if(root == null)
            throw new Exception("No Elements in Stack");
        else {
            int data = -1;
            if(root.next==null) {
                data = root.data;
                root = null;
                return data;
            }

            StackNode temp = root;
            while(temp.next.next != null)
                temp = temp.next;
            data = temp.next.data;
            temp.next = null;
            return data;
        }
    }

Hope it helps.