current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite

I would like to know if this is a great stack implementation. I created my own exception. Was that a bad idea or unnecessary? Should I have just thrown a IndexOutOfBounds exception instead of my own.

import java.util.*;
public class Stack2{
    private int[] stack;
    private int size;

    public Stack2(){
        stack = new int[10];
        size = 0;
    }

    public Stack2(int height){
        if(height <= 0){
            throw new EmptyStackException();
        }
        stack = new int[height];
        size = 0;
    }

    public void add(int value){
        if (size == stack.length){
            throw new StackOverflowException();
        }
        stack[size] = value;
        size++;
    }


    public int pop(){
        if(size == 0){
            throw new EmptyStackException();
        }
        size--;
        return stack[size + 1];
    }

    public int peek(){
        if(size == 0){
            throw new EmptyStackException();
        }
        return stack[size - 1];
    }

    public boolean isEmpty(){
        if(size == 0){
            return true;
        }
        return false;
    }

    public int getSize(){
        return this.size;
    }

    class StackOverflowException extends RuntimeException{
        public StackOverflowException(){
            super("Nothing can be added to the stack. The stack was full and has overflowed");
        }

        public StackOverflowException(String message){
            super(message);
        }
    }
}
share|improve this question

closed as off-topic by Vogel612, Pimgd, t3chb0t, Tunaki, Dannnno Jul 27 '16 at 11:28

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Vogel612, Pimgd, t3chb0t, Tunaki, Dannnno
If this question can be reworded to fit the rules in the help center, please edit the question.
1  
Have you tested this code for correctness? Why is there a StackOverflowException class that you never use? Would you like to post your EmptyStackException class? – 200_success♦ Jul 26 '16 at 18:01
    
I use the StackOverflowException class in the add method. EmptyStackException is a part of the java API. – jillian Jul 26 '16 at 18:10
1  
It has been demonstrated in an answer, that this code does not behave as expected (add(3);pop() returns 0). As such I'm voting to close this question as off-topic because the code is not working as intended. For more information, please see the help center. Thanks – Vogel612 Jul 27 '16 at 7:47
    
No, it works. I fixed it. – jillian Jul 27 '16 at 11:19
    
@jillian we don't allow editing code after an answer has been given – Dannnno Jul 27 '16 at 11:27
up vote 3 down vote accepted

Your stack returns 0 instead of 3 on this simple case:

public static void main(String[] args) {
    Stack2 s = new Stack2();
    s.add(3);
    System.out.println(s.pop());
}

Traditionally, the stack operations are called "push" and "pop". It's unusual to have "add" and "pop".

The isEmpty() method would be better written as:

public boolean isEmpty(){
    return size == 0
}

Throwing ArrayIndexOutOfBoundsException would be worse, since it would result in a "leaky" abstraction. The user shouldn't know that the stack is actually backed by an array.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.