HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Proper Stack Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
properimplementationstack

Problem

I want to know if this stack is implemented properly. Previously I was told a Linked List contains an inner class that represents a node and the outta class that represents the structure. Does that same idea apply to stacks and linked list? I am assuming not because they are so simple. Here is my implementation. Let me know if this is correct.

public class Stack{
  private int size;
  private int[] stack;

  public Stack(int height){
    stack = new int[height];
    size = 0;
  }

  public void push(int value){
    if(size == 0){

      stack[size] = value;
      size++;

    }

    else if(size == stack.length){
      System.out.println("The stack is full. The value cannot be added to the stack");
    }

    else {
    stack[size] = value;
    size++;
    }

  }

  public void pop(){

    if(size == 0){
      System.out.println("The stack is empty. Nothing can be removed from the stack");
    }
    else {
      size--;
      System.out.println(stack[size] + " was removed from the stack");

    }

  }

  public void peek(){

    if(size == 0){
      System.out.println("There is nothing to peek. The stack is empty");

    }
    else
      System.out.println(size);
      System.out.println("The value: " + stack[size -1]+ " is currently at the top of the stack");

  }

  public void isEmpty(){

    if(size == 0)
      System.out.println("The stack is empty");
    else
      System.out.println("The stack is not empty");

  }
}

Solution

Stack is a black hole from other code

The first impression I get is you can't get anything back once you put it in the Stack.

peek and pop both return void, so once you've put your value in with push that's the end of it. I know your methods output to the console, but that's pretty useless to the rest of your program.

When you're writing things like data structures try to put the user interaction outside of the class you're modelling. Both for input and output. This will encourage you to define methods that actually allow you to use the data structure in the way that you would in a normal program. With that in mind, peek at least should return a value. isEmpty should return a bool. Depending on your approach, pop could also return the value popped, or it could simply remove the item (however if you go down this approach I would expect it to throw a exception if you pop from an empty stack).

I would also be expecting push to throw an exception if the stack is full. At the moment, the client has no idea that their value hasn't been added to the stack.

Context

StackExchange Code Review Q#135952, answer score: 6

Revisions (0)

No revisions yet.