2
\$\begingroup\$

I just started learning java and wrote this variable length array as my first program. I wanted people's opinion on how "java" this code is. For example coding conventions, idioms that I could have used or any improvements that could be made.

MyClass.java:

package myproject;

public class MyClass {

    public static void main(String[] args) {        
        MyArray array = new MyArray();
        
        for (int i = 0; i < 100; i++) array.Push(i);
        
        for (int i = 0; i < array.Size(); i++) {
            System.out.println(array.Get()[i]);
        }

    }
}

MyArray.java:

package myproject;

public class MyArray {
    public MyArray() {
        array = new int[size];
    }
    
    public MyArray(int size) {
        this.size = size;
        array = new int[size];
    }
    
    public int[] Get() {
        return array;
    }
    
    public void Push(int number) {
        if (currentIndex >= size) {
            IncreaseArraySize();
        }
        array[currentIndex] = number;
        currentIndex++;
    }
    
    int Size() {
        return currentIndex;
    }
    
    private void IncreaseArraySize(){
        if (size == 0){
            size++;
        }
        else {
            size *= 2; 
        }
        
        int newArray[] = new int[size];
        for (int i = 0; i < array.length; i++) {
            newArray[i] = array[i];
        }
        array = newArray;
    }
    
    private int size = 0;
    private int currentIndex = 0;
    private int[] array;
}

I was also kind of sceptical of this line : array = newArray; at first but the program seems to work fine.

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Advice 1

public MyArray() {
    array = new int[size];
}

Above, size is 0, so you are allocating a zero-length array. Useless. Consider this:

private static final int INITIAL_CAPACITY = 10; // Or some other reasonable constant but 10.

public MyArray() {
    array = new int[INITIAL_CAPACITY];
}

Advice 2

    ...
    private int size = 0;
    private int currentIndex = 0;
    private int[] array;
}

You put your fields after everything else. The conventional portion of a Java file for fields is right after static constants and right before constructors:

private static final int INITIAL_CAPACITY = 10;

private int size = 0;
private int currentIndex = 0;
private int[] array;

public MyArray() {
    ...
}

Advice 3

Once again, the fields:

private int size = 0;
private int currentIndex = 0;
private int[] array;

JVM sets int fields to zero by default. Consider this:

private int size;
private int currentIndex;
private int[] array;

Advice 4

public int[] Get() {
    return array;
}

Wrong. Please, don't expose data structure related internals to the outside world.

Advice 5

Method names must come in camelCase, not in PascalCase.

Advice 6

public MyArray(int size) {
    this.size = size;
    array = new int[size];
}

What?! Your MyArray (no pun intended) is of size of size at the very beginning? In the state where all the "elements" are 0?

Advice 7 (ignore)

Actually, it seems to me that you don't need to keep size at all. Judging from your implementation we have always size == array.length. Ditch the size and rely on array.length; much easier to follow.

Advice 8

I suggest you add a get(int) method for accessing the data.

Advice 9

In IncreaseArraySize(), you could use System.arraycopy. Most likely, it will copy faster on large arrays.

\$\endgroup\$
3
  • \$\begingroup\$ For advice 1, my intention was for the array to be empty if size isn't specified in the constructor and calling Push would increase the size anyways. I also don't understand advice 6, apart from the fact that it does actually makes sense to completely get rid of size. \$\endgroup\$ Commented Sep 11, 2021 at 15:24
  • 1
    \$\begingroup\$ The array size needs to be tracked separately for IncreaseArraySize to work. Otherwise you have to resize the array every time an element is added or removed, which is very inefficient. \$\endgroup\$
    – Eric Stein
    Commented Sep 11, 2021 at 17:06
  • \$\begingroup\$ @EricStein In this program, array size is stored at currentIndex. The variable called size is actually the capacity, which is always the same as the array.length. I.e. your analysis is correct (you need two sizes), but due to bad naming, it doesn't apply here. \$\endgroup\$
    – mdfst13
    Commented Sep 11, 2021 at 17:12
4
\$\begingroup\$
    private int size = 0;
    private int currentIndex = 0;

Given how you use these, better names would be

    private int capacity = 0;
    private int size = 0;

But as @coderodde already noted, it would make more sense to get the capacity from array.length rather than maintain it separately.

I would also rename array to data, but that's a more arguable point.

\$\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.