3
\$\begingroup\$

The following example shows how to implement stack by creating user defined push() method for entering elements and pop() method for retrieving elements from the stack.

public class MyStack {
  private int maxSize;
  private long[] stackArray;
  private int top;

  public MyStack(int s) {
    maxSize = s;
    stackArray = new long[maxSize];
    top = -1;
  }

  public void push(long j) {
    stackArray[++top] = j;
  }

  public long pop() {
    return stackArray[top--];
  }

  public long peek() {
    return stackArray[top];
  }

  public boolean isEmpty() {
    return (top == -1);
  }

  public boolean isFull() {
    return (top == maxSize - 1);
  }

  public static void main(String[] args) {
    MyStack theStack = new MyStack(10);
    theStack.push(10);
    theStack.push(20);
    theStack.push(30);
    theStack.push(40);
    theStack.push(50);

    while (!theStack.isEmpty()) {
      long value = theStack.pop();
      System.out.print(value);
      System.out.print(" ");
    }

    System.out.println("");
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ What happen if I pop and the stack is empty ? What if I push and the stack was at maxSize ? \$\endgroup\$ – Marc-Andre Jun 13 '14 at 18:30
  • \$\begingroup\$ If you try to 'pop' when the stack is empty, it will return an error. Therefore, there needs to be another stack operation to do the test, implemented as 'boolean isEmpty()' which tests whether a stack is empty, and will return true if the stack is empty. \$\endgroup\$ – user44869 Jun 13 '14 at 18:35
5
\$\begingroup\$

The stack that you have implemented (I guess for an assignment) is too limited because of the following:

  1. It does not support generic types. Since every container class in Java can store any object, I would suggest you to incorporate that facility in your data structure as well.
  2. It has only a limited storage capacity, due to the use of Java array. It would be better to use a container class that can grow dynamically, such as an ArrayList, LinkedList or a Vector.
  3. You should properly handle the boundary condition of popping from an empty stack.
  4. Try to test your main function with a real-life application, such as evaluating a post-fix expression or checking valid parenthesization etc.
\$\endgroup\$
  • \$\begingroup\$ ArrayList is not thread-safe. \$\endgroup\$ – rds Jun 15 '14 at 20:06
  • 3
    \$\begingroup\$ @rds And how would it change the thread-safety of the OP's code? \$\endgroup\$ – maaartinus Jun 15 '14 at 20:27
  • 1
    \$\begingroup\$ "3. You should properly handle the boundary condition of popping from an empty stack." - They do. Throwing an exception is best. \$\endgroup\$ – maaartinus Jun 15 '14 at 20:34
  • 3
    \$\begingroup\$ @maaartinus An EmptyStackException would be better than ArrayOutOfBoundExcetpion, but indeed throwing an exception is the right thing to do. @Debasis Vector is deprecated, don't use it for anything other than crying for his existence. \$\endgroup\$ – Marc-Andre Jun 16 '14 at 2:47
3
\$\begingroup\$
  • Your code is not thread-safe
  • Why do you limit the size of the stack. Instead, you could use a Vector with the same logic, or a Collections.synchronizedList() of an Arraylist.
  • And by the way, what's wrong with java.util.Stack (which is a Vector, precisely)?
  • It's bad practice to have two variables for the same thing. You should not have maxSize, that's exactly stackArray.length
  • It would be better to throw your own StackFullException and StackEmptyException rather than the ArrayIndexOutOfBoundsException. A stack is not an array. (Even though it's perfectly fine to have an implementation based on array)
  • MyStack is a terrible name
  • Except if you don't want to pay the price of autoboxing/autounboxing, I think it's a pity you don't have MyStack<T>
  • Javadoc is missing.
\$\endgroup\$
  • 1
    \$\begingroup\$ "And by the way, what's wring with java.util.Stack?" Nearly everything. It's synchronized and it offers methods not usually considered to belong to a stack. \$\endgroup\$ – maaartinus Jun 15 '14 at 20:31
  • 1
    \$\begingroup\$ Avoid the old collection classes like Vector and Hashtable. If you need thread-safe, you're better of with the concurrent versions. \$\endgroup\$ – David Harkness Jun 15 '14 at 23:50
  • \$\begingroup\$ @DavidHarkness Right, I added the use of Collections.synchronizedList \$\endgroup\$ – rds Jun 16 '14 at 9:07

Your Answer

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