2
\$\begingroup\$

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;
 }
\$\endgroup\$
3
\$\begingroup\$

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)
\$\endgroup\$
  • 1
    \$\begingroup\$ 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 . \$\endgroup\$ – E.Etem Sep 28 '18 at 10:10
2
\$\begingroup\$

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");
}
\$\endgroup\$
  • \$\begingroup\$ Method names in Java start with a lowercase letter. We also put the curly brace on the same line, not a newline. \$\endgroup\$ – Eric Stein Sep 29 '18 at 13:56
  • \$\begingroup\$ Updated, waiting for it to be reviewed. \$\endgroup\$ – Markus Deibel Sep 30 '18 at 17:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.