1
\$\begingroup\$

I have implemented a stack using simple array, dynamic array and linked list. I am requesting a review of this code.

Interface for stack:

/**
 * Interface for stack
 */
public interface Stack<T> { 
public void push(final T data);
public T pop();
public boolean isStackEmpty();
public boolean isStackFull();
public T peek();
}

Implement stack using simple array:

/**
 * Inplement stack using array.
 */
public class Stack1<T> implements Stack<T>{ 
private int sizeOfStack;
private T stackArray[];
private static int tos; //top of stack

public Stack1(final int sizeOfStack) {
    tos = -1;
    stackArray = (T[])new Object[sizeOfStack];
    this.sizeOfStack = sizeOfStack;
}

/**
 * pushes item in the stack.
 * 
 * @param data
 */
public void push(final T data) {        
    if(isStackFull()) {
        throw new RuntimeException("stack is full cannot push");
    }
    stackArray[++tos] = data;       
}

/**
 * gets item from stack
 */
public T pop() {
    if(isStackEmpty()) {
        throw new RuntimeException("stack is empty");
    }
    return stackArray[tos--] ;
}

/** 
 * Checks if the stack is empty or not
 * 
 * @return boolean
 */
public boolean isStackEmpty() {     
    return (tos == -1);
}

/**
 * Check if the stack is full .
 * 
 * @return boolean
 */
public boolean isStackFull() {      
    return (tos == sizeOfStack - 1);
}

/**
 * Looks at the top element in the stack
 */
public T peek() {
    return stackArray[tos];
}
}

Implement stack using dynamic array:

/**
 * Dynamic array implementation.
 */
public class Stack2<T> implements Stack<T>{

private int tos ;
private int size;
private T[] array;

public Stack2(final int size) {
    tos = -1;
    this.size = size;
    array = (T[])new Object[size];
}

/**
 * pushes item in the stack.
 * 
 * @param data
 */
public void push(final T data) {            
    if(isStackFull()) {
        T[] array2 = (T[])new Object[size * 2];
        System.arraycopy(array, 0, array2, 0, array.length);
        array = array2;
    }
    array[++ tos] = data;
}

/**
 * gets item from the stack.
 */
public T pop() {
    return array[tos -- ];
}

/**
 * check if stack is empty
 */
public boolean isStackEmpty() {
    return (tos == -1);
}

/**
 * check if stack is full
 */
public boolean isStackFull() {
    return (tos == size - 1 );
}

/**
 * check the element from top of stack.
 */
public T peek() {
    return array[tos];
}
}

Implement stack using linked list:

/**
* Implement stack using LinkedList
*/
public class Stack3<T> implements Stack<T> {

Node<T> tos;

public Stack3() {
    tos = null;
}

public void push(final T data) {    
    Node<T> newNode = new Node<T>(data);
    newNode.setNext(tos);
    tos = newNode;
}

public T pop() {        
    if(tos != null) {
        final T item = tos.getData();
        tos = tos.getNext();
        return item;
    }
    return null;
}

public boolean isStackEmpty() {
    return (tos == null );
}

public boolean isStackFull() {
    return false;
}

public T peek() {
    return tos.getData();
}
}

Test cast for stack:

public class StackTest {

// test stack using array
@Test
public void test() {        
    Stack1<Integer> stack = new Stack1<Integer>(7); 
    stack.push(10);
    stack.push(20);
    stack.push(30);
    stack.push(40);
    stack.push(50);
    stack.push(5);
    stack.push(1);
    printStack(stack);
}

//test stack using dynamic array
@Test
public void test2() {       
    Stack2<Integer> stack = new Stack2<Integer>(7); 
    stack.push(10);
    stack.push(20);
    stack.push(30);
    stack.push(40);
    stack.push(50);
    stack.push(5);
    stack.push(1);
    stack.push(100);
    stack.push(200);
    stack.push(300);
    stack.push(400);
    stack.push(500);
    stack.push(600);
    stack.push(700);
    printStack(stack);
}

//test stack using linkedlist
@Test
public void test3() {       
    Stack3<Integer> stack = new Stack3<Integer>();  
    stack.push(10);
    stack.push(20);
    stack.push(30);
    stack.push(40);
    stack.push(50);
    stack.push(5);
    stack.push(1);
    stack.push(100);
    stack.push(200);
    stack.push(300);
    stack.push(400);
    stack.push(500);
    stack.push(600);
    stack.push(700);
    printStack(stack);
}

public void printStack(final Stack<Integer> stack) {
    while(!stack.isStackEmpty()) {
        System.out.println(stack.pop());
    }
}
}
\$\endgroup\$
1
\$\begingroup\$

General

Your implementations are both a fixed size and a dynamic sizes stack that behave differently. Either choose to let them behave similarly (so add a fixed size), or change the interface.

Tests

Your tests don't really test, they just print. You should add assertions. Also, test 'good path' and 'wrong path' (for example: pushing when the Stack is full)

Naming

Please use proper names instead of short ones.

  • tos is not readable, topOfStack is much clearer
  • isStackFull. What does that mean? I would expect that you cannot add more elements if the stack is full.

Magic values

You use a -1 to indicate the stack is empty. I would add a constant with the name TOP_OF_EMPTY_STACK and document it

Exceptions

You use RuntimeException. This is kind of a last resort then implementing an interface without Exceptions. You should use properly type exceptions like StackFullException

Documentation

You add javadocs (good!). But you add them to the implementation, but not to the interface. Please add them to the interface and just @See in the implementation if nothing changes.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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