1
\$\begingroup\$

This is my array implementation on stack in Java. It involves push, pop, get and growth when limit of array size is reached.

I am a self-taught programmer starting to learn data structures and algorithms.

I am looking for

  1. any recommendation on conventions
  2. an honest opinion on how bad this piece of code if this is an interview.
  3. anything wrong and room for improvement.

I wrote this code myself, not looking at any other similar questions on Code Review.

public class stackAlist{
    int[] data;
    int size;
    static int growthmultipler = 1;

    public stackAlist(){
        data = new int[2*growthmultipler];
        size = 0;
    }

    public void push(int value){
        if(this.size == 0){
            this.data[0] = value;
            this.size += 1;
            this.growthmultipler = 1;
            }
        else if(this.size == 2*growthmultipler){
            growthmultipler += 1;
            stackAlist newlist = new stackAlist();
            newlist.data = new int[2*growthmultipler];
            System.arraycopy(this.data, 0, newlist.data, 0, this.size);
            this.data = newlist.data;
            this.data[size] = value;
            this.size += 1;
        }
        else{
        this.data[size] = value;
        this.size += 1;
        }
    }

    public void pop(){
        this.data[size-1] = 0;
        this.size = this.size-1;
    }

    public void get(){
        int i;
        for(i =0; i < this.size; i++){
            System.out.println(data[i]);
        }
    }

    public static void main(String[] args){
        stackAlist a = new stackAlist();
        a.push(1);
        a.push(2);
        a.get();
        a.pop();
        a.get();
        a.push(3);
        a.push(4);
        a.get();
    }
}
\$\endgroup\$
0
\$\begingroup\$

First of all, you are using a static variable (growthmultipler) inside the instance of a class. This is fatal if you have more than one instance of this class. Because you always use 2*growthmultipler, you don't need it. Use +=2 instead.

You don't need to prealloc data in the constructor. It will be done if it's needed.

You don't need to handle "size == 0" separately. You only have to handle "size >= length".

You don't have to create a new class if you want to grow data.

In pop() you should test to not get negative (and throw an exception). And normally a pop function returns the value.

In summary it might look like this:

public class stackAlist{
    int[] data;
    int size;

    public stackAlist(){
        size = 0;
        data = new int[size];
    }

    public void push(int value){
        if(size>=data.length) {
            int [] ndata = new int[data.length+2];
            System.arraycopy(data, 0, ndata, 0, size);
            data = ndata;
        }
        data[size] = value;
        size += 1;
    }

    public int pop() {
        int ret=0;
        if(size>0) {
            size -= 1;
            ret = data[size];
            data[size] = 0;
        }
        return ret;
    }
    .....
\$\endgroup\$
  • \$\begingroup\$ Exceptions exist for a reason; the use of a "special" return value should be well documented. \$\endgroup\$ – Solomon Ucko Nov 19 '18 at 12:30
  • \$\begingroup\$ I do think it should fail early, though. \$\endgroup\$ – Solomon Ucko Nov 19 '18 at 12:38
0
\$\begingroup\$

A couple things to add:

  1. Classes should use CamelCase and should preferably have descriptive names. stackAlist would be better named ArrayStack.
  2. You might want to have a Stack interface that ArrayStack implements.
  3. You might want to extract the resizing code into a separate method, and optionally make it public and/or take parameters.
  4. In your pop method, there's no need to zero out the popped items.
\$\endgroup\$
  • \$\begingroup\$ Thank you for all answers. I am kind of stuck at thinking about why I need a Stack interface that ArrayStack implements. Because I don't know why, I can't think of how to do it. An Interface should be used when there are multiple classes/data types involved. Are you saying that I should make ArrayStack generic so that it can accept all data types? Kinda confused here, please help, thanks. \$\endgroup\$ – Carch Nov 20 '18 at 8:19
  • \$\begingroup\$ Generics might be good too. Also, having an interface isn't necessary, but it can allow you to decouple it from code that uses it. \$\endgroup\$ – Solomon Ucko Nov 20 '18 at 11:41

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.