8
\$\begingroup\$

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, my tests may not be as comprehensive as they should.

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
}
\$\endgroup\$
1
  • 4
    \$\begingroup\$ Other suggestion - try making this class actually implement the Stack interface. \$\endgroup\$ Commented Oct 12, 2012 at 21:35

3 Answers 3

7
\$\begingroup\$

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
\$\endgroup\$
9
\$\begingroup\$
  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.

\$\endgroup\$
6
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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