My removeDownTo() method does not show the correct output

Question

I am doing this exercise my professor told the class to do. "Write a method public static void removeDownTo (StackX stack, long ob): It pops all values off the stack down to but not including the first element it sees that is equal to the second parameter. If none are equal, leave the stack empty."

This is my code.

StackX class:

public class StackX {
    private static int maxSize; // size of stack array
    private static long[] stackArray;
    private int top; // top of stack
    //--------------------------------------------------------------
    public StackX(int s) // constructor
    {
        maxSize = s; // set array size
        stackArray = new long[maxSize]; // create array
        top = -1; // no items yet
    }
    //--------------------------------------------------------------
    public void push(long j) // put item on top of stack
    {
        stackArray[++top] = j; // increment top, insert item
        /**      if (isFull() ) {
                  System.out.println("Push error: Stack is full. Push failed.");
              } else {
                  stackArray[++top] = j;
              } */
    }
    //--------------------------------------------------------------
    public long pop() // take item from top of stack
    {
        return stackArray[top--]; // access item, decrement top
        /**      if(!isEmpty()) {
                  return stackArray[top--];
              } else {
                  System.out.println("Error: Stack is empty. Returning -1");
                  return -1;
              }
            */
    }

    //--------------------------------------------------------------
    public long peek() // peek at top of stack
    {
        return stackArray[top];
    }
    //--------------------------------------------------------------
    public boolean isEmpty() // true if stack is empty
    {
        return (top == -1);
    }
    //--------------------------------------------------------------
    public boolean isFull() // true if stack is full
    {
        return (top == maxSize - 1);
    }
}

StackApp class:

public class StackApp
   {

  public static void removeDownTo(StackX stack, long n) {
      long x;
      while(!stack.isEmpty()) {
          x = stack.peek();
          stack.pop();
          if(stack.isEmpty()) {
              if(x==n) {
                  stack.push(x);
              }
          }
      }
  }

   public static void main(String[] args)
      {
      StackX theStack = new StackX(10);  // make new stack
      theStack.push(20);               // push items onto stack
      theStack.push(40);
      theStack.push(60);
      theStack.push(80);
    //  theStack.push(16);
     // theStack.push(10);

      while( !theStack.isEmpty() )     // until it's empty,
         {                             // delete item from stack
         long value = theStack.pop();
         System.out.print(value);      // display it

         System.out.print(" ");

         }  // end while

      System.out.println("");
      removeDownTo(theStack, 60);
      System.out.print("");
      }  // end main()
   }  // end class StackApp ##

This is the output it shows: 80, 60, 40, 20.

However, I think the output this exercise is asking for is to get 60, 40, 20. What am I doing wrong?

Answer 1

So, I'll try to make this answer as much educational as possible.

First off all, static members (or fields, as you prefer):

private static int maxSize;
private static long[] stackArray;

Static members are shared among all instances of StackX, so, say you instantiate a second StackX object, the first one will "see" and use the new maxSize and stackArray values. This isn't what you want. For now it's ok, as you're using a single StackX object.

Second, variable/parameter naming. Avoid names such as x, j, n, they just say nothing about their role in the program. Use more expressive terms such as size, value, downTo.

Third, comments. I see you like writing a lot of comments, that's good. But remember to use the right tool. To express what a method or class does, use JavaDoc. Leave line comments for internal details only.

Fourth, try to use final as much as possible on members (fields), variables and parameters. Remember that mutability (that is, the degree you allow your object to change during execution) is usually bad.

Fifth, use exceptions to signal something isn't working right. For example, the stack is full and someone tries to insert a new value? Throw an exception with a descriptive message.

Sixth, always format your code! No matter it's a couple lines or a million, always format it and avoid excessive line lenght.

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

    public StackX(final int size) {
        maxSize = size;
        stackArray = new long[maxSize];
        top = -1;
    }

    /** Puts an item on top of the stack. */
    public void push(final long value) {
        if (isFull()) {
            throw new UnsupportedOperationException("Stack is full");
        }

        stackArray[++top] = value;
    }

    /** Takes an item from top of the stack. */
    public long pop() {
        if (isEmpty()) {
            throw new UnsupportedOperationException("Stack is empty");
        }

        return stackArray[top--];
    }

    /** Peeks an item at top of the stack. */
    public long peek() {
        return stackArray[top];
    }

    /** Returns {@code true} if stack is empty. */
    public boolean isEmpty() {
        return top == -1;
    }

    /** Returns {@code true} if stack is full. */
    public boolean isFull() {
        return top == maxSize - 1;
    }
}

---

public class Main {
    public static void main(final String[] args) {
        final StackX stackX = new StackX(5);
        stackX.push(20);
        stackX.push(40);
        stackX.push(60);
        stackX.push(80);

        removeDownTo(stackX, 60);

        // Prints remaining elements
        while (!stackX.isEmpty()) {
            System.out.println(stackX.pop());
        }

        System.out.println(stackX.isEmpty());
    }

    private static void removeDownTo(
            final StackX stack,
            final long downTo) {
        while (!stack.isEmpty()) {
            if (stack.peek() == downTo) {
                return;
            }

            stack.pop();
        }
    }
}