HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Generic ArrayList

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericarrayliststackoverflow

Problem

Can anyone recommend improvements to the compress() method?

public class ArrayList  {

private Object[] array;
public static final int DEFAULT_SIZE = 20;
private int size = 0;

public ArrayList(){
    this(DEFAULT_SIZE);
}

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

public void add(E element){
    ensureCapacity();
    array[size++] = element;
}

public E remove(int index){
    if(index>=size || index  size){throw new RuntimeException("Invalid index");}
    E element = (E) array[index];
    return element;
}

public int size(){
    return this.size;
}

private void ensureCapacity(){
    if(size < array.length){ return;}
    resize();
}

private void resize(){
    Object[] temp = new Object[array.length*2];
    copy(array,temp);
    array = temp;
}

private void copy(Object[] src, Object[] dest){
    if(dest.length< src.length){throw new RuntimeException(src+ " cannot be copied into "+dest);}
    for(int i=0;i<src.length; i++){
        dest[i] = src[i];
    }   
}

private void compress(){
    int skipCount =0;
    for(int i=0;i < size && i<array.length; i++){
        if(array[i]==null){
            ++skipCount;                
        }
        array[i]=array[i+skipCount];
    }
}

}

Solution

General

There are a few things in here....

-
You allow users to add a null value in the add() method, but you will then 'compress' it later if you remove something.

-
you have an off-by-one error on the get() (but not remove() ) methods... it should be >= size, and not > size.

-
compress() is only called from remove(). I would recommend removing the compress() method and making remove just compress 1 value:

public E remove(int index){
    if(index>=size || index < 0 ){throw new RuntimeException("Invalid index");}
    E element = (E) array[index];
    --size;
    System.arraycopy(array, index + 1, array, index, size - index);
    array[size] = null;
    return element;
}


Alternatively, pass in an int value to the compress method.

If you do the above you will correctly support null values.

-
Although it is tempting, don't call your class ArrayList.... it competes with the standard class name, and will produce problems. Use something like 'MyArrayList'.

Generics

What you have right now is fine with respect to the Generics.

If you want to be a bit more correct, you can use a Class-type initializer, and use that to create correctly typed arrays, and avoud the generic (T)array[index] casting later.... but that is a pain, and requires passing Class clazz in as the constructor.

Code Snippets

public E remove(int index){
    if(index>=size || index < 0 ){throw new RuntimeException("Invalid index");}
    E element = (E) array[index];
    --size;
    System.arraycopy(array, index + 1, array, index, size - index);
    array[size] = null;
    return element;
}

Context

StackExchange Code Review Q#44889, answer score: 4

Revisions (0)

No revisions yet.