Stack data structure implementation in Java

Question

I recently made a data structure, stack implementation in Java and I would like feedback on my code to improve it (other than commenting/documenting my code).

public class Stack {

    private int topIndex, maxSize;
    private int[] stack;

    public Stack(int size) {
        maxSize = size;
        stack = new int[maxSize];
        topIndex = -1;
    }

    public int maxSize() {
        return maxSize;
    }

    public int size() {
        return topIndex + 1;
    }

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

    public boolean isFull() {
        return (topIndex + 1) == maxSize;
    }

    public void push(int value) {
        if (!isFull()) {
            topIndex++;
            stack[topIndex] = value;
        }
    }

    public int peek() {
        if (!isEmpty()) {
            return stack[topIndex];

        } else {
            return 0;
        }
    }

    public void pop() {
        if (!isEmpty()) {
            topIndex--;
        } 
    }

    public static void main(String[] args) {

    }

}

Answer 1

I'd like to give some thoughts which spring to my mind when I read your given code. Now, you have an implementation specific for primitive ints. A more advanced step would of course be to implement a generic Stack like the one in the JDK, just mentioning it.

First up, I like that you use clear variable names, and that you reuse methods like isEmtpy \ isFull. Kudos for that! As an aside, you do not need the maxSize variable, as it is the same as stack.length.

It is a fixed size stack, so it does mean you have to know beforehand how much elements it would take up, in order to not waste space (or run out of it). Perhaps for a follow up exercise you could try and make it dynamically resizable?

Now, for the push method. I would not let it silently succeed if the stack is already full. Seeing as it is a fixed size stack, I would probably opt to throw an Exception, because to me it looks like a programming error. A possible second idea could be to return a boolean, signaling succes or failure (though I don't think it is the better solution).

Same with peek. It now silently succeeds and returns 0, which is very well a valid value which could reside in the stack (EmptyStackException exists by the way!). The boolean solution would not work here of course. Also, I personally do not think the extra new line is an improvement, on the contrary.

Finally, to me pop means taking off the top element of the stack. So why not return it?

Answer 2

Most points have already been covered by Koekje. However I don't think declaring more than one variable per line is a good idea. It's easy to misread it if you happen to be low on coffee.
Consider for example int i, j = 1; which might lead some people to mistakingly believe both variables are being initialized. You also lose the ability to comment on individual variables if need be.
Further reading [1][2]

I also disagree with your decision to add additional vertical space in your peek function. Normally vertical space suggests a break in the flow/logic. In your example however the if statement is one logical unit and should be treated as such.

Answer 3

Overall a good looking code. Here is some of my feedback:

private int[] stack; use generics.

public Stack(int size) make sure size is greater than 0.

stack = new int[maxSize]; a smarter way would be to dynamically expand or compress.

public int maxSize() { better name would be 'getMaxSize()'

if (!isFull()) { You may want to throw an exception or return false if you fail to add to stack. Otherwise you could return true.

return 0; 0 could be a valid value in stack. again use boolean/exception for such cases.

public void pop() { pop method should never be void.