LinkedList Stack Implementation

Question

I'm new to the programming and I want to ask a question about my linkedlist stack implementation if it's correct and if it meets the requirements of the linkedlist stack implementation. If there is any suggestion I'm glad to hear...

public class DynamicStack {
  private class Node {
     private Object item;
     private Node next;

     Node(Object item){
      this.item = item;
      this.next = null;
     }

     Node(Object item, Node nextNode){
      this.item = item;
      this.next  = nextNode;
     }
  }

 private int count;
 private Node top;

 public DynamicStack() {
   this.top = null;
   this.count = 0;
 }



 public void push(Object item) {
   if(top == null) {
      top = new Node(item);
   }else {
      Node newNode = new Node(item,top);
      top = newNode;
   }
   count++;
 }

 public Object peek() {
   if(top == null) {
      throw new NoSuchElementException("Underflow Exception");
   }else {
      return top.item;
   }
 }
 public Object pop() {
   Node currentNode = top;
   if(top == null) {
      throw new NoSuchElementException("Underflow Exception");
   }else {
      Node nextNode = top.next;
      top = null;
      top = nextNode;
   }
   count--;
   return currentNode.item;
 }

Answer 1

This looks to me like it will work as intended. I have a few suggestions for improvement though:

• You keep the count variable up to date but never use it's value for anything. You can either remove it or provide a size() method.
• You might change your push() method to

public void push(Object item){
   top = new Node(item, top);
   count++;
}

If top is null, you set node.next to null in the Node constructor anyways. - If you do this, you can subsequently delete the Node(Item) constructor

• I personally would make the item and next member variables in Node final to prevent accidental mutations
• (Instead of accepting Object as your items you might want to introduce Generics into your class. But this is an advanced topic and is maybe too hard for you as a beginner)

Answer 2

One thing that you could try is moving the null-check for peek and pop to its own method. In this way you can eliminate the if/else blocks and reduce indentation which is better for reading. Also the name gives you a quick indication what is going to happen on the error without the need to analyse the whole statement in the if. For this code it's a very small improvement, but if the code blocks get larger, it's rather significant.

public Object peek() {
    breakIfTopisNull();

    return top.item;
}

public Object pop() {
    Node currentNode = top;

    breakIfTopIsNull()

    Node nextNode = top.next;
    top = null;
    top = nextNode;

    count--;
    return currentNode.item;
}

private void breakIfTopIsNull() {
    if(top == null) 
        throw new NoSuchElementException("Underflow Exception");
}