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;
 }
}
asked Jan 27, 2017 at 2:28
\$\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\$ Commented 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.

answered Jan 27, 2017 at 7:02
\$\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\$ Commented Jan 27, 2017 at 7:46
  • \$\begingroup\$ @mtj Fair enough, yet, I am afraid, that is a matter of taste. \$\endgroup\$ Commented 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\$ Commented Jan 27, 2017 at 7:51
  • \$\begingroup\$ @SLR You are most welcome. If you are satisfied with an answer, consider accepting it. \$\endgroup\$ Commented 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.

answered Dec 4, 2018 at 19:34
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hi, I just wanted to congratulate you on your first review, it's valuable. \$\endgroup\$ Commented 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];
}
answered Jul 23, 2018 at 1:30
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.