Implementation of Non-Generic Stack in Java

Question

I'm starting out to work on DS in Java for the first time and thought that some inputs will get me going to more data structures. I implemented a stack which is non-generic. I followed some of the posts and realized that the top pointer should not be at -1. I know this is a very basic implementation (probably the worst). The LIFO part is working well. I am looking for suggestions on what are some ways to improve this code other than making this to be Generic Stack.

Code:

import java.util.*;


//******************************* Stack Demo Class **********//

class StackDemo{
  int stackarray[] , top;

  // Constructor

  StackDemo(int size){
    stackarray = new int[size+1];
    top = 0;
    System.out.println("The size of stack array is "+ (stackarray.length-1));
  }

  // *********************PUSH******************************//

  void push(int ele){
    if(top == stackarray.length - 1){
      System.out.println("Stack Overflow");
    }
    else{
      top++;
      stackarray[top] = ele;
    }
  } // Push Method


  // *********************POP******************************//

  int pop(){
    int lastElement = 0;
    if(top == 0){
      System.out.println("Stack is Underflow");
    }
    else{
      lastElement = stackarray[top];
      top--;
    }
    return lastElement;
  }  // Pop Method


  // *********************DISPLAY******************************//

  void display(){
    if(top == 0){
      System.out.println("Nothing to Display");
    }
    for (int i = 1; i<=top ; i++){
      System.out.print(stackarray[i] +"\t");
    }
    System.out.println("");
  } // Display Method

}

// *****************************Main Class*******************//

public class Stacks{
  public static void main(String args[]){
    StackDemo sd = new StackDemo(10);
    Scanner s = new Scanner(System.in);
    while(true){
      System.out.println("Enter your choice : \n 1.Push \n 2.Pop \n 3.Display \n 4.Exit");
      int choice = s.nextInt();
      switch(choice){
        case 1:
          System.out.println("Enter an Element to push");
          int pushElement = s.nextInt();
          sd.push(pushElement);
          break;
        case 2:
          int popElement = sd.pop();
          System.out.println("Popped Element is "+popElement);
          break;
        case 3:
          sd.display();
          break;
        case 4:
          System.exit(0);
          break;
        default:
          System.out.println("Enter valid choice");
      }  // Switch Closing
    } // While Closing
  } // Main method
} // Main class

Answer 1

Thanks for sharing your code,

Avoid Wild Imports

You import

import java.util.*;

but the only thing you use from the util package is Scanner, so why not just

import java.util.Scanner;

Now it's obvious to anyone reading what you'll be using from util.

Counting Starts at 0

stackarray = new int[size+1];

in your constructor, If I make a stack of size 10, I'm actually making one of size 11!

You're also accommodating for your +1 size in your constructor with

if(top == stackarray.length - 1){
   ...
}

if you just initialised your array as new int[size];

then this could look like

if(top == stackarray.length){
   ...
}

At the moment you're offsetting in 2 places, when in fact you don't need to do this anywhere! This is making the code more difficult for other people to follow.

In your display method

for (int i = 1; i<=top ; i++){
    ...
}

you can just start this at i = 0, starting at 1 is very unnatural.

this should be re-written like

for (int i = 0; i < top ; i++){
     ...
}

Unusual Behaviour

right now, if I pop() an empty stack, it will return 0. This would be extremely unusual behaviour for an empty data structure. It wouldn't really be empty anymore would it?

  int pop(){
    int lastElement = 0;
    if(top == 0){
      System.out.println("Stack is Underflow");
    }
    else{
      lastElement = stackarray[top];
      top--;
    }
    return lastElement;
  }  // Pop Method

instead of printing "Stack is Underflow", why not throw a new EmptyStackException(msg). That's what it's for after all.

In your push method, you print a message if the stack is full. Again I would suggest throwing an appropriate exception here, or even better, you could resize the array to accommodate the new element. (This would be great practice for making an ArrayList implementation.)

What's Missing?

Your stack is missing an isEmpty() or empty() method, this should just return true/false for if the stack is empty or not. This is extremely simple to implement so you shouldn't have any problems with this!

Your stack is missing a peek() method, this should show the next element, without removing it.

Why Stop There?

Some other methods you might like to add for practice could be,

public boolean contains(int element) // if the lement is present in the stack

public int numberOf(int element) // occurrences of a given element

public int smallest() // the smallest element in the stack

public int largest()

public int totalSum()

public int average()

Hopefully this review was useful for you, keep it up!