Array Implementation of Stack

Question

I've implemented the basic logic of a stack data structure. How can I make this code more generic? Is there any better way to display the stack contents?

StackMethods.java

import java.util.Arrays; public class StackMethods { private int top; int size; int[] stack ; public StackMethods(int arraySize){ size=arraySize; stack= new int[size]; top=-1; } public void push(int value){ if(top==size-1){ System.out.println("Stack is full, can't push a value"); } else{ top=top+1; stack[top]=value; } } public void pop(){ if(!isEmpty()) top=top-1; else{ System.out.println("Can't pop...stack is empty"); } } public boolean isEmpty(){ return top==-1; } public void display(){ for(int i=0;i<=top;i++){ System.out.print(stack[i]+ " "); } System.out.println(); } } 

StackReviseDemo.java

public class StackReviseDemo { public static void main(String[] args) { StackMethods newStack = new StackMethods(5); newStack.push(10); newStack.push(1); newStack.push(50); newStack.push(20); newStack.push(90); newStack.display(); newStack.pop(); newStack.pop(); newStack.pop(); newStack.pop(); newStack.display(); } } 

Answer 1

Good

• Naming of parameters and methods follow the naming convention

Bad

• not checking input parameter for constructor
• not listening to previous answers ( pop() returning nothing )
• not using braces {} for every if..else statement

Additional

• int size should be final as it won't be changed.
• Instead of display() you can override toString() as this is more common.
• Instead of writing to System.out you should throw an exception.
• give your variables some space to breathe. E.g top=top+1; would be more readable if written like so top = top + 1;

Answer 2

Few comments:

• The name StackMethods is not the best name for this, you should rather pick a name that describes your entity, eg: Stack

• Your class only works for integers, where Stack is an ADT that should accept various types, so consider using generics instead

  class Stack<T>{ private T[] stackArray; } 

• the fields : size, and stack have package access, and this means classes within the same package can modify them, declare them as private instead.

• You should throw an unchecked exception in push method if the stack is full rather than printing something to the console

Answer 3

I would suggest to use an LinkedList inside of your StackMethods.java. It already has methods for to push and pop.

A simple example, without your size-limits, etc.:

public class StackReviseDemo { public static void main(String[] args) { LinkedList<Integer> newStack = new LinkedList<Integer>(); newStack.push(10); newStack.push(1); newStack.push(50); newStack.push(20); newStack.push(90); display(newStack); newStack.pop(); newStack.pop(); newStack.pop(); newStack.pop(); display(newStack); } private static void display(LinkedList<Integer> newStack) { boolean first = true; LinkedList<Integer> reversedStack = new LinkedList<>(newStack); Collections.reverse(reversedStack); for (Integer integer : reversedStack) { if (first) first = false; else System.out.print(" "); System.out.print(integer); } System.out.println(); } } 

Answer 4

@Heslacher's and @Sleiman Jneidi's answers are really good. Just in case you want to have a list implementation, you can compose List inside your custom-defined Stack ADT. This way, only stack methods will be exposed outside, excluding that of List's.

public class Stack<T> { private List<T> contents; public Stack() { contents = new ArrayList<>(); } public void push(T item) throws NullPointerException, IllegalArgumentException{ contents.add(item); //add to tail } public T pop() throws IndexOutOfBoundsException { return contents.remove(contents.size() - 1); //remove from tail } @Override public String toString() { return contents.toString(); } }