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)

• E.Etem: Thanks for the suggestions. Yes there were two more methods for size() and clear() ,but i just didn't added them to the post. About to push() method your right, thanks. Also i saw few Generics implementations, but they are really like you say for an advanced and i was trying to implement a basic structure to learn how the stack works and to use it in the future. Also i'm sorry, because i can't +rep your comment, it was really useful - because of my reputation. Thanks again .

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");
}

• Eric Stein: Method names in Java start with a lowercase letter. We also put the curly brace on the same line, not a newline.
• Markus Deibel: Updated, waiting for it to be reviewed.