Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Just coded up a simple Stack using an Array in Java for practice. Other than the lack of JavaDoc (still need to learn that part), did I cover all the bases?

import java.util.Iterator;

public class ArrayStack<T> implements Iterable<T> {
    private T[] stack;
    private int top = 0;  //Position of the top of the stack

    //Constructor
    public ArrayStack(int size) {
        //Generic array creation isn't allowed, using cast
        stack = (T[]) new Object[size];
    } //End Constructor

    //Add item to top of stack
    public void push(T item) {
        //If you've reached top of stack, double it's size
        if (top == stack.length) {
            resize(stack.length *2);
        }

        stack[top++] = item;
        StdOut.println(stack[top-1] + " Added to stack"); //For debugging
    } //End push

    //Removes item from top of stack
    public T pop() {
            T toReturn = stack[--top];
            stack[top] = null;  //Garbage cleanup

            //Reduce size of array if it is less than 1/3rd full
            if (top < stack.length/3 && top > 10) {
                resize(stack.length/2);
            }
            return toReturn;
    } //End pop

    //Simple true or false if the stack is empty or not
    public boolean isEmpty() {
        return (top == 0);
    } //End isEmpty

    //Returns the iterator
    public Iterator<T> iterator() {
        return new StackIterator();
    } //End iterator

    public class StackIterator implements Iterator<T> {
        private int i = top; //Increment starting at top of stack

        @Override
        //True until i is the bottom of the stack
        public boolean hasNext() {
            return (i > 0);
        } //End hasNext

        @Override
        //Returns the items in the stack starting from top
        public T next() {
            return stack[--i];
        } //End next

        @Override
        //Not implemented
        public void remove() { }
    } //End StackIterator

    //Creates a new array of the passed in size and copy values into it
    private void resize(int size) {
        StdOut.println("Resizing stack to " + size);  //For debugging
        //New array for values
        T[] tempStack = (T[]) new Object[size];

        //Copies values into new array
        for (int i = 0; i < top; i++) {
            tempStack[i] = stack[i];  
        }
        //Assign stack to the new array
        stack = tempStack;
        StdOut.println("Stack size is now " + stack.length); //For debugging

    } //End resize

    //Test client
    public static void main(String[] args) {
        int size = 5;
        //Have to use wrapper Integer since it's expecting an object
        ArrayStack<Integer> myStack  = new ArrayStack<Integer>(size);  

        int choice = 3;
        while (choice != 0) {
            StdOut.println();
            StdOut.println("1: Push a number to top of stack");
            StdOut.println("2: Pop  a number from top of stack");
            StdOut.println("3: Display stack");
            StdOut.println("0: Quit");
            StdOut.print("Choice: ");
            choice = StdIn.readInt();

            if (choice == 1) {
                StdOut.print("Number to add: ");
                Integer toAdd = StdIn.readInt();
                myStack.push(toAdd);
            }

            else if (choice == 2) {
                StdOut.println(myStack.pop() + " removed from stack");  
            }

            else if (choice == 3) {
                for (Integer x : myStack) {
                    StdOut.println(x + " ");
                }
            }
        } //End while
    } //End main
} //End ArrayStack
share|improve this question
    
The contract of Iterator requires that next() throws a NoSuchElementException if it is empty. Throwing any other exception is just plain wrong, as that would be an unpleasent surprise for any client code relying on the documentation of Iterator. –  Landei Feb 12 at 13:16

1 Answer 1

up vote 0 down vote accepted

For starters, comment pairs such as //Constructor ... //End Constructor are redundant and actually makes the code a little harder to read.

Your debugging statements can be better served using a standard logging library that lets you log statements categorized by severity, e.g. org.slf4j.Logger.debug().

Instead of creating your inner class StackIterator, you can consider in-lining the implementation in the following way:

public Iterator<T> iterator() {
    return new Iterator<T>(){
        private int i = top; //Increment starting at top of stack

        @Override
        public boolean hasNext() {
            return (i > 0);
        }

        @Override
        public T next() {
            return stack[--i];
        }

        @Override
        public void remove() { 
            throw new UnsupportedOperationException();
        }
    };
}

You can also see that remove() throws an UnsupportedOperationException(), to make the (non-)usage more apparent.

Last but not least, I'm not sure how much reinventing-the-wheel do you intend/expect to do here, but you can also look into using Arrays.copyOf() to handle the array resizing.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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