Stack implementation, array-based

Question

My understanding is that Stack possesses LIFO property. And, based upon that I have written the following program:

// This is a sample program to demonstrate push and pop // functionality in Stack in Java. package collections; public class Stack_Array { private static final int capacity = 3; int[] arr = new int[capacity]; int top = -1; public void push(int pushedElement) { if(top >= capacity) { System.out.println("StackOverflow !!"); } else { top++; arr[top] = pushedElement; } } public void pop() { if(top>=0) { top--; } else { System.out.println("Stack underflow !!"); } } public void printStack() { System.out.println("Elements in Stack are: "); for(int i = top; i >=0; i--) { System.out.println(arr[i]); } } public static void main(String[] args) { Stack_Array s1 = new Stack_Array(); s1.push(23); s1.push(21); s1.push(14); s1.pop(); s1.printStack(); } } 

Please let me know if it is the correct approach. Or, is there a better way to implement the same

Answer 1

I like the consistency of your braces and indentation - well done!

The name Stack_Array should be StackArray, since as member names are camelCase, type names ought to be PascalCase, not Upper_Snake_Case.

---

You'll want to implement a constructor so that the capacity of the object can be parameterized, and then the calling code can look like this:

StackArray foo = new StackArray(42); foo.push(bar); 

The underlying capacity static field would need to be changed a bit though; you'll want it to be an instance field, so that each object can have its own independent value. I'd keep it final though, because it's not a value you want to be able to modify during the lifetime of an instance - beyond the constructor, nothing should be allowed to modify that value.

---

You'll want to throw an appropriate exception in exceptional situations:

if(top >= capacity) { System.out.println("StackOverflow !!"); 

A Stack is a data structure; other code will rely on its correct behavior, and expect a number of errors to occur in certain specific situations - for example when the caller attempts to push an element into an instance that already contains as many items as it can hold.

That said, I'd prefer a Stack implementation that simply increases its capacity as items are pushed. In any case, printing "StackOverflow !!" isn't appropriate. Making the method return a bool that the calling code can evaluate to determine whether the push/pull succeeded or not, would be a better alternative if you really want a fixed capacity.

---

Printing to the console isn't the job of a data structure, printStack should be replaced by a method that returns a new data structure that contains a copy of the internal array. Such a toArray method would leave it up to the calling code to decide whether they want to print the items to the console, or dump them into a log file, or whatever.

---

Your implementation is pretty bare-bones, you'll want to look at the java.util.Stack<E> members and possibly dig into generics. The C# System.Collections.Generics.Stack<T> class could also be an inspiration.

The pop method would be expected to return the popped item. A peek method is missing, to "peek" at the next item without popping it out of the stack.

---

Other observations

Between push and pop, the two have an if conditional, but one is no-op in the positive branch, while the other is no-op in the negative branch (/else block). It would be better to decide whether you test for the error condition [to throw an error and fail early] or if you test for the success condition [and throw an error in the else block] - it's usually better to fail early and reduce the indentation level:

public void foo() { if ({error condition}) { throw new SomeException(); } doSomething(); } 

Watch the spacing on either side of comparison operators:

for(int i = top; i >=0; i--) { 

Give it the same whitespace as for the = assignment operator:

for(int i = top; i >= 0; i--) {