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;
}
}
3 Answers 3
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.
-
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– mtj2017年01月27日 07:46:28 +00:00Commented Jan 27, 2017 at 7:46
-
\$\begingroup\$ @mtj Fair enough, yet, I am afraid, that is a matter of taste. \$\endgroup\$coderodde– coderodde2017年01月27日 07:47:56 +00:00Commented 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– coderodde2017年01月27日 07:51:36 +00:00Commented Jan 27, 2017 at 7:51
-
\$\begingroup\$ @SLR You are most welcome. If you are satisfied with an answer, consider accepting it. \$\endgroup\$coderodde– coderodde2017年01月29日 19:58:01 +00:00Commented Jan 29, 2017 at 19:58
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.
-
1\$\begingroup\$ Hi, I just wanted to congratulate you on your first review, it's valuable. \$\endgroup\$IEatBagels– IEatBagels2018年12月04日 20:53:40 +00:00Commented Dec 4, 2018 at 20:53
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];
}
java.util.List
, as that provides a common understood interface and helps you know what should be implemented. \$\endgroup\$