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.

I have created a simple array-based Stack class with some methods like the actual Stack in Java.

I am testing this for mistakes, but since I am learning Java and my tests may not be as comprehensive as they should. So I decided to ask help from CodeReview users.

import java.util.*;

public class SJUStack<E> {
    // Data Fields
    private E[] theData;
    private int topOfStack = -1;
    private static final int INITIAL_CAPACITY = 10;
    private int size = 0;
    private int capacity = 0;

    // Constructors
    public SJUStack(int initCapacity) {
        capacity = initCapacity;
        theData = (E[]) new Object[capacity];
    }

    public SJUStack() {
        this(INITIAL_CAPACITY);
    }

    // Methods
    public E push(E e) {
        if(size == capacity) {
            reallocate();
        }
        theData[size] = e;
        size++;
        topOfStack++;

        return e;
    } // End push(E e) method

    public E peek() {
        if(empty()) {
            throw new EmptyStackException();
        }
        return theData[topOfStack];
    } // End peek() method

    public E pop() {
        E result = peek();
        theData[topOfStack] = null;
        size--;
        topOfStack--;
        if(size <= (capacity/4) && capacity >= INITIAL_CAPACITY) {
            shrink();
        }
        return result;
    } // End pop() method

    public boolean empty() {
        return size == 0;
    } // End empty() method

    private void reallocate() {
        capacity *= 2;
        theData = Arrays.copyOf(theData, capacity);
    } // End reallocate() method

    private void shrink() {
        capacity /= 2;
        theData = Arrays.copyOf(theData, capacity);
    } // End shrink() method

    public String toString() {
        return Arrays.toString(theData);
    } // End toString() method

    public int size() {
        return size;
    } // End size() method
}
share|improve this question
2  
Other suggestion - try making this class actually implement the Stack interface. –  Clockwork-Muse Oct 12 '12 at 21:35

3 Answers 3

up vote 4 down vote accepted

I don't see any obvious logic mistakes, points for that. On the other hand some redundant and non-standard ways of coding in java.

  1. Drop either size or topOfStack members, (topOfStack == size - 1)
  2. Drop capacity capacity is same as theData.length
  3. Method name: isEmpty (more concise with java standard collections)
  4. Use data instead of theData
share|improve this answer

1). I suggest to use following method name: isEmpty() instead of empty()

2). If your Java version > 1.5 you should use @Override for your toString() method.

3). Do you really need the the article in class field name like theData?

4). You don't need both size and topOfStack variables. theData.size is enough.

share|improve this answer

topOfStack is always one less than size, so you don't need that variable. Just replace all instances with size-1. Similarly, capacity could be replaced with theData.length.

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.