4
\$\begingroup\$

I am new to Java and am trying to learn coding. Here is my implementation of a stack and a queue without the help of library functions. Please provide your suggestions on design, coding style and algorithm.

Stack.java

import java.util.NoSuchElementException;
import java.lang.UnsupportedOperationException;

public class Stack<T>
{
    private int size;
    private int topPointer=-1;
    private T[] stackArray;

    public Stack()
    {
        this.size=10;
        stackArray=(T[]) new Object[size];
    }

    public Stack(int size)
    {
        this.size=size;
        stackArray=(T[]) new Object[size];
    }

    public void push(T element)
    {
        if(isFull())
        {
            throw new UnsupportedOperationException("Stack is full");
        }
        else
        {
            topPointer++;
            stackArray[topPointer]=element;
            System.out.println(element+" pushed to stack");
        }
    }

    public T pop()
    {
        if(isEmpty())
        {
            throw new UnsupportedOperationException("Stack is empty");
        }
        else
        {
            T element=stackArray[topPointer];
            topPointer--;
            return element;
        }
    }

    public boolean isFull()
    {
        if(topPointer==size-1)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    public boolean isEmpty()
    {
        if(topPointer<0)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    public T peek()
    {
        if(isEmpty())
        {
            throw new UnsupportedOperationException("Stack is empty");
        }
        else
        {
            return stackArray[topPointer];
        }
    }

    public int search(T element)
    {
        if(!isEmpty())
        {
            for(int i=0;i<size;i++)
            {
                if(stackArray[i]!=null && stackArray[i].equals(element))
                {
                    return size-i;
                }
            }
        }
        throw new NoSuchElementException("Element not found : "+element.toString());
    }
}

Queue.java

import java.util.NoSuchElementException;
import java.lang.UnsupportedOperationException;

public class Queue<T>
{
    private int size;
    private int rarePointer=-1;
    private T[] queueArray;

    public Queue()
    {
            this.size=10;
            queueArray=(T[]) new Object[size];
    }

    public Queue(int size)
    {
        this.size=size;
        queueArray=(T[]) new Object[size];
    }

    public void enQueue(T element)
    {
        if(isFull())
        {
            throw new UnsupportedOperationException("Queue is full");
        }
        else
        {
            rarePointer++;
            queueArray[rarePointer]=element;
            System.out.println(element+" pushed to queue");
        }
    }

    public T deQueue()
    {
        if(isEmpty())
        {
            throw new UnsupportedOperationException("Queue is empty");
        }
        else
        {
            T element=queueArray[0];
            for(int i=0;i<size-1;i++)
            {
                queueArray[i]=queueArray[i+1];
            }
            rarePointer--;
            return element;
        }
    }

    public boolean isFull()
    {
        if(rarePointer==size-1)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    public boolean isEmpty()
    {
        if(rarePointer<0)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    public T peek()
    {
        if(isEmpty())
        {
            throw new UnsupportedOperationException("Queue is empty");
        }
        else
        {
            return queueArray[0];
        }
    }

    public int search(T element)
    {
        if(!isEmpty())
        {
            for(int i=0;i<size;i++)
            {
                if(queueArray[i]!=null && queueArray[i].equals(element))
                {
                    return i+1;
                }
            }
        }
        throw new NoSuchElementException("Element not found : "+element.toString());
    }
}
\$\endgroup\$
6
\$\begingroup\$

Stack

Boolean logic:

An expression "a == b" or "topPointer == size - 1" is already a boolean. Thus your isFull and isEmpty methods essentially read:

if(true) {
    return true;
}
else {
    return false;
}

replace this with the direct expression:

public boolean isEmpty() {
    return topPointer < 0;
}

// same for isFull

Search method:

Search needs to loop to topPointer, not to size. As topPointer is already smaller than a starting i = 0 for an empty stack, you can leave out the precheck for emptyness:

for(int i = 0; i <= topPointer; i++) {
    if(stackArray[i] != null && stackArray[i].equals(element)) {
         return topPointer - i; // provided that you mean to say: this many elements away from the top.
    }
}
throw new NoSuchElementException(...);

Style:

Spaces around operators and a different brace style would be java standard as used by most of the rest of the world. See the code-snippets I posted.

For your Queue implementation the feedback is exactly the same.

\$\endgroup\$
  • \$\begingroup\$ Thank you so much. I have made these modifications to code. \$\endgroup\$ – SLR Jan 26 '17 at 19:52
5
\$\begingroup\$

Three points.

You call your Stack class Stack. That will cause confusion because there is already a Stack class in the Java library. Better to use a different name to avoid the clash: MyStack.

You have a magic number, 10, in your default constructor. Using magic numbers is generally bad practice. Assign the value to a named variable, and let the variable name describe the number.

You have two separate constructors which effectively do the same job and both take a single int parameter (implicit for the default constructor). That means that any future changes might have to be repeated twice, once in each constructor. There is a Java idiom which allows one constructor to call another constructor. You may not have done this in class yet, so feel free to ignore it, but I have included it in the code below.

public class MyStack<T> {

    private final static int DEFAULT_SIZE = 10;

    private int size;
    private int topPointer = -1;
    private T[] stackArray;

    // Default constructor.
    public MyStack() {
        // Call sized constructor to do the work.
        this(DEFAULT_SIZE);
    }

    // Sized constructor.
    public MyStack(int size) {
        this.size = size;
        stackArray = (T[]) new Object[size];
    }

    // Rest of MyStack class ...

} // end class MyStack.
\$\endgroup\$
  • \$\begingroup\$ I have read about this(DEFAULT_SIZE) in theory.. but while implementing I could not put it to use :(. Thank you for reminding me. Can you please point out if there are any other such improvements. \$\endgroup\$ – SLR Jan 26 '17 at 19:48
  • \$\begingroup\$ You have the same thing in your Queue implementation. Basically, when you have a specific number that means something, put it into a final variable and use the variable. That way you only have to change the code in one place if you want a default size of 20 instead. You also do not have to search through all the code to check each individual occurrence of "10" to see if is relevant or not. That helps maintainability. \$\endgroup\$ – rossum Jan 26 '17 at 20:06

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.