Array-based stack

Question

Description:

An array-based Stack class. Since this class is designed using an array the size of the Stack is limited to whatever the program gives you. The key reason was not to use any outside help like Arrays.copyOf(array, size) to increase size etc. (In future, LinkedStack will solve size problem). MAX_SIZE is made public to let a user know about MAX_SIZE and size() returns current size of the Stack. Return value int is used to make sure all push/insertions are successful on Stack.

Right now I'm just looking for suggestions to improve my code before I push it to my GitHub account.

package Stack;

public class StackArray
{
   public static final int MAX_SIZE = 1040;   // MAX size of a Stack
   private int topLook;                       // current element on Stack
   private int stackElements[];               // array to store Stack elements

public StackArray()                        // default constructor creates a Stack with capacity of 50
{
    topLook = -1;                          // initially no elements on the Stack
    stackElements = new int[MAX_SIZE];    // an array of MAX_SIZE gets created
}

public int size()                          // returns current size of the Stack
{
    return (topLook + 1);
}

public boolean emptyCheck()                // returns true if this Stack has no elements in it, false otherwise
{
    return (topLook == -1);
}

public boolean fullCheck()                 // returns true if this Stack is full, false otherwise
{
    return (topLook == MAX_SIZE - 1);
}

public int top()                           // returns top element of the Stack, -1 otherwise
{
    if (!emptyCheck())
    {
        return stackElements[topLook];
    }
    return -1;
}

public int push(int value)                 // push passed argument on the Stack. 1 indicates insertion was successful, -1 otherwise
{
    if (!fullCheck())
    {
        stackElements[++topLook] = value;
        return 1;
    }
    return -1;
}

public int pop()                           // returns top element of the Stack and remove it, -1 otherwise
{
    if (!emptyCheck())
    {
        return stackElements[topLook--];
    }
    return -1;
}
}

Answer 1

The methods named emptyCheck() and fullCheck() are ambiguous. They should be named isEmpty() and isFull() respectively.

Good luck!

Answer 2

Formatting

You should clean up your indention. The methods should be indented to the same level as the fields and it is common in Java to place the opening bracket on the same line as the matching statement. For example:

public class StackArray {

   public static final int MAX_SIZE = 1040;
   private int topLook;
   private int stackElements[];

   // Note the indentation and position of the bracket here.
   public StackArray() {
      topLook = -1;                          
      stackElements = new int[MAX_SIZE];
   }

   // ...

}

Comments

Comments such as

public static final int MAX_SIZE = 1040;   // MAX size of a Stack

that just repeat what the line does are pointless.

Also you can minimize the number of comments by choosing better variable/method names. Take for example:

private int topLook;                       // current element on Stack

If you call the variable currentElement (or topElement), the comment becomes unnecessary.

Some of your comments are simply wrong (or you didn't update them) such as:

// default constructor creates a Stack with capacity of 50

Important are comments that give new information such as:

topLook = -1;                          // initially no elements on the Stack

Because it's not obvious that -1 means that the stack is empty.

You also should consider placing your comments before the line/method they concern, instead of after it. It makes the comments better readable and gives you more space to write longer comments if needed. Also for the methods you should consider using Javadoc style comments.

Implementation

There are some problems with the functionality which maybe due to simplified instructions:

top() and pop() can't distinguish between an empty stack and a value of -1. Also returning having push() return an int (1 or -1) isn't very nice.

There are better ways to do either of these things, but maybe the teacher hasn't introduced the necessary concepts yet. Maybe speak to the teacher about it.

Answer 3

Broken functionality

I think that your code is broken: if you push(-1) and then pop() it, how do you know if the stack is empty or -1 is the actual value you got?

There is a problem here with returning 1/-1 to indicate if a push/pop is successful. 1/-1 and valid values for elements in the array. The usual approach for these cases is to throw exceptions. If the stack provides a size() method, and MAX_SIZE is public, the client can avoid these exceptions. If you still want to avoid exceptions, perhaps a boolean would be better than 1/-1.

Other issues

Following Java's naming conventions, you should name your package with lower case, stack.

Also, as mentioned by TwiN, the names for emptyCheck() and fullCheck() can be improved. Method names should be verbs, and specifically, boolean methods should ask a question that can be answered with yes/no.

Then, the comments are misplaced (they should be above, not to the right), and many of them are unneeded because they're so obvious. Comments like "default constructor creates a Stack with capacity of 50" are wrong; that's one of the reasons why you should avoid comments if possible, that is, if the code is clear enough.