2
\$\begingroup\$

Below is my implementation of dynamic array without help of library functions. Kindly provide your suggestions on design, coding style and algorithm.

MyDynamicArray.java

import java.util.NoSuchElementException;

public class MyDynamicArray<T>
{
    private int positionPointer=0;
    private int arraySize;
    private T[] dynamicArray;
    private static final int DEFAULT_ARRAY_SIZE=10;

    public MyDynamicArray()
    {
        this(DEFAULT_ARRAY_SIZE);
    }

    public MyDynamicArray(int arraySize)
    {
        this.arraySize=arraySize;
        dynamicArray=(T[]) new Object[arraySize];
    }

    public void addElement(T element)
    {
        adjustSize();
        dynamicArray[positionPointer]=element;
        positionPointer++;
    }

    public void addElementAtNode(int index, T element)
    {
        if(index<positionPointer)
        {
            dynamicArray[index]=element;
        }
        else
        {
            addElement(element);
            throw new ArrayIndexOutOfBoundsException("index "+index+" is greater than the size of array "+(positionPointer-1)+" \nElement added to end of array..");
        }
    }

    private void adjustSize()
    {
        if(positionPointer==arraySize)
        {
            increaseSize();
        }
        else if(positionPointer==(arraySize/4-1))
        {
            decreaseSize();
        }
    }

    private void increaseSize()
    {
        T[] tempArray=(T[]) new Object[2*arraySize];
        for(int i=0;i<positionPointer;i++)
        {
            tempArray[i]=dynamicArray[i];
        }
        dynamicArray=tempArray;
        arraySize=2*arraySize;
    }

    private void decreaseSize()
    {
        T[] tempArray=(T[]) new Object[(arraySize/4)];
        for(int i=0;i<positionPointer;i++)
        {
            tempArray[i]=dynamicArray[i];
        }
        dynamicArray=tempArray;
        arraySize=arraySize/4;
    }

    public int searchElement(T element)
    {
        for(int i=0;i<positionPointer;i++)
        {
            if(dynamicArray[i].equals(element))
            {
                return i;
            }
        }
        throw new NoSuchElementException("Element not found : "+element.toString());
    }

    public T getElementAtIndex(int index)
    {
        if(index<positionPointer)
        {
            return dynamicArray[index];
        }
        else
        {
            throw new ArrayIndexOutOfBoundsException("index "+index+" is greater than the size of array "+positionPointer);
        }
    }

    public void removeElement(T element)
    {
        int index=searchElement(element);
        if(index>0)
        {
            removeElementAtIndex(index);
        }
    }

    public void removeElementAtIndex(int index)
    {
        if(index<positionPointer)
        {
            for(int i=index;i<positionPointer-1;i++)
            {
                dynamicArray[index]=dynamicArray[index+1];
            }
            dynamicArray[positionPointer-1]=null;
            positionPointer--;
            adjustSize();
        }
        else
        {
            throw new ArrayIndexOutOfBoundsException("index "+index+" is greater than the size of array "+positionPointer);
        }
    }

    public int size()
    {
        return positionPointer;
    }

}
\$\endgroup\$
1
  • \$\begingroup\$ I know you are reinventing-the-wheel, but still consider implementing java.util.List, as that provides a common understood interface and helps you know what should be implemented. \$\endgroup\$
    – CAD97
    Jan 27, 2017 at 5:59

3 Answers 3

3
\$\begingroup\$

Advice 1

if(positionPointer==arraySize)

You should have always one space before and after a binary operator:

if(positionPointer == arraySize)

Advice 2

For Java, more conventional way of writing blocks is

if (...) {
    ...
}

instead of

if (...) 
{
    ...
}

Advice 3

Omit the arraySize and use dynamicArray.length instead.

Advice 4

private int positionPointer=0;

Just write

private int positionPointer;

since JVM initializes integer fields to zero by default.

Advice 5

You use positionPointer for keeping track of the number of elements in your data structure. For that very reason, I suggest you rename it to size.

Advice 6

public void addElementAtNode(int index, T element)
{
    if(index<positionPointer)
    {
        dynamicArray[index]=element;
    }
    else
    {
        addElement(element);
        throw new ArrayIndexOutOfBoundsException("index "+index+" is greater than the size of array "+(positionPointer-1)+" \nElement added to end of array..");
    }
}

Above, if the index is correct, you basically set an element instead of adding it. I suggest you rename it to set. Also, it seems strange what you do in the case if index is invalid.

Advice 7

In your increaseSize, you can just say:

dynamicArray = Arrays.copyOfRange(dynamicArray, 0, 2 * dynamicArray.length);

Also, same applies to decreaseSize:

dynamicArray = Arrays.copyOf(dynamicArray, dynamicArray.length / 4);

Advice 8

public int searchElement(T element)
{
    for(int i=0;i<positionPointer;i++)
    {
        if(dynamicArray[i].equals(element))
        {
            return i;
        }
    }
    throw new NoSuchElementException("Element not found : "+element.toString());
}

If the array contains a null value, if(dynamicArray[i].equals(element)) will throw. Also, conventional lists return the value -1 in case of missing element instead of throwing NoSuchElementException:

public int searcElement(T element) {
    for (int i = 0; i < positionPointer; ++i) {
        if (Objects.equals(element, dynamicArray[i])) {
            return i;
        }
    }

    return -1;
}

Advice 9

In removeElementAtIndex, instead of

dynamicArray[positionPointer-1]=null;
positionPointer--;

you can write

dymaicArray[--positionPointer] = null;

Advice 10

You make sure that the indices are not too large. However, you must make sure that they are not negative either.

Hope that helps.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ Good advice, but I beg to differ on #4: I think it greatly improves understandability of code, if initializations are done explicitly where the programmer wants to express that he really means to use this value. This is even true, if that certain value chances to be the VMs default value for the given data type. \$\endgroup\$
    – mtj
    Jan 27, 2017 at 7:46
  • \$\begingroup\$ @mtj Fair enough, yet, I am afraid, that is a matter of taste. \$\endgroup\$
    – coderodde
    Jan 27, 2017 at 7:47
  • \$\begingroup\$ @mtj And yes, you probably have a point: it is easy to implement the JVM such that integer fields are initialized only once even if that initialization value is not zero. \$\endgroup\$
    – coderodde
    Jan 27, 2017 at 7:51
  • \$\begingroup\$ @SLR You are most welcome. If you are satisfied with an answer, consider accepting it. \$\endgroup\$
    – coderodde
    Jan 29, 2017 at 19:58
4
\$\begingroup\$

In almost all methods for example in this one instead of

public void removeElement(T element)
{
    int index=searchElement(element);
    if(index>0)
    {
        removeElementAtIndex(index);
    }
}

should be

public void removeElement(T element)
{
    int index=searchElement(element);
    if(index >= 0)
    {
        removeElementAtIndex(index);
    }
}

Because otherwise the first element will get you an error.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hi, I just wanted to congratulate you on your first review, it's valuable. \$\endgroup\$
    – IEatBagels
    Dec 4, 2018 at 20:53
3
\$\begingroup\$

In removeElementAtIndex, instead of

for(int i=index;i<positionPointer-1;i++)
{
    dynamicArray[index]=dynamicArray[index+1];
}

I think it should be

for(int i=index;i<positionPointer-1;i++)
{
    dynamicArray[i]=dynamicArray[i+1];
}
\$\endgroup\$

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.