3
\$\begingroup\$

I have started learning data structures and have implemented my own stack implementation:

package com.algo.stack;

import java.io.Serializable;
import java.lang.reflect.Array;

public class MyStack<T extends Serializable> {

    private T[] myArray = null; // the array is the stack content
    private int arraySize = 0; // holds the size counter


    /*
     * Initialize stack for its size and generic type
     */
    public MyStack(Class<T> c,int stackSize){ 

        @SuppressWarnings("unchecked")
        final T[] a = (T[]) Array.newInstance(c , stackSize);
        this.myArray = a;
    }

    /**
     * The push method, which adds
     * numbers in internal array 
     * @param obj
     * @throws Exception
     */
    public void push(T obj) throws Exception{
        if(this.arraySize < this.myArray.length){
            this.myArray[this.arraySize++] = obj;
        }else{
            throw new Exception("MyStack is Full, need to pop");
        }

    }

    /**
     * pops or removes elements from
     * the internal array in LIFO fashion
     * @return
     * @throws Exception
     */
    public T pop() throws Exception{
        if(this.arraySize <= 0){
            throw new Exception("Nothing to pop, stack is empty");
        }
        return this.myArray[--this.arraySize]; 
    }

    /**
     * Returns the total size 
     * @return
     */
    public int size(){
         return this.arraySize;
    }

    public static void main(String[] args){
        MyStack<Integer> stack1 = null;
        MyStack<String> stack2 = null;
        try{
            stack1 = new MyStack<Integer>(Integer.class, 3);
            stack1.push(12);
            stack1.push(24);
            stack1.push(36);

            while(stack1.size() > 0){
                System.out.println(stack1.pop());
            }

            System.out.println("\n");

            stack2 = new MyStack<String>(String.class, 4);

            stack2.push("Leo");
            stack2.push("Tom");
            stack2.push("Nancy");
            stack2.push("Julean");


            while(stack2.size() > 0){
                System.out.println(stack2.pop());
            }
        }catch(Exception e){
            e.printStackTrace();
        }
    }
}

It's working fine and the output is:

36
24
12


Julean
Nancy
Tom
Leo

Please review my rough implementation and let me know of any flaws and how I can improve.

\$\endgroup\$
1
  • 3
    \$\begingroup\$ There are a lot of existing questions on self-made stack implementations with many good reviews you could learn from. \$\endgroup\$ – janos Apr 23 '17 at 17:28
7
\$\begingroup\$
T extends Serializable

Why? There doesn't seem to be a good reason to do this.


throw new Exception("MyStack is Full, need to pop");

Your professor probably told you to do it like this, but it'd at the very least be an interesting exercise to see if you can resize the array to hold more, rather than letting it be full. It's not terribly difficult, either.


@SuppressWarnings("unchecked")

These are almost always unnecessary; you theoretically should be able to rewrite your code such that they're unnecessary.

However, if I'm remembering my Java correctly, that's not possible in this case, so it's better to eliminate the warning, so that the "real" ones are more visible.


this.myArray[this.arraySize++]

This code, though its behavior is well-defined, is not what you should be writing. Split it into two lines. The two statements -- the set and the increment -- already connected by being very nearly the sole contents of a method. Putting the increment in the brackets just makes it more likely that the increment won't be seen. I, for one, didn't see it until I took a closer look, because it appeared to be incorrect.

You don't get an award for fewer lines. Every second you don't need to spend re-reading your code later is a second you can use to implement new, cool features, and every second people reading your code don't spend on unclear syntax is a second they can use to find real bugs.


It's unfortunate that you can't remove the Class<T> passed in to the constructor, but that's the way it is, so good job for minimizing its impact.


Space and indent things consistently. This:

new MyStack<Integer>(Integer.class, 3);

is not consistent with this:

Array.newInstance(c , stackSize);

Now, your response to any of these could be "meh, it's a project for school, so I don't really care about that", to which my response is this:

School is supposed to teach you how to code well, in part by drilling how to do that into your head through practice. If you don't practice writing code well, you'll find yourself stuck in habits of writing code that works, but nobody can read after three months. If you practice writing code well -- as in, readable, clean code, not necessarily bug-free -- starting as soon as possible, you'll save yourself a lot of headache debugging later.

\$\endgroup\$
0

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.