My implementation of dynamic array
up vote
1
down vote
favorite
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;
}
}
java array
asked Jan 27 '17 at 2:28
SLR
6917
add a comment |
up vote
1
down vote
favorite
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;
}
}
java array
asked Jan 27 '17 at 2:28
SLR
6917
I know you are reinventing-the-wheel, but still consider implementingjava.util.List, as that provides a common understood interface and helps you know what should be implemented.
– CAD97
Jan 27 '17 at 5:59
add a comment |
up vote
1
down vote
favorite
up vote
1
down vote
favorite
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;
}
}
java array
asked Jan 27 '17 at 2:28
SLR
6917
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;
}
}
java array
java array
asked Jan 27 '17 at 2:28
SLR
6917
asked Jan 27 '17 at 2:28
SLR
6917
asked Jan 27 '17 at 2:28
SLR
6917
asked Jan 27 '17 at 2:28
SLR
6917
asked Jan 27 '17 at 2:28
SLR
6917
6917
I know you are reinventing-the-wheel, but still consider implementingjava.util.List, as that provides a common understood interface and helps you know what should be implemented.
– CAD97
Jan 27 '17 at 5:59
add a comment |
I know you are reinventing-the-wheel, but still consider implementingjava.util.List, as that provides a common understood interface and helps you know what should be implemented.
– CAD97
Jan 27 '17 at 5:59
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.– CAD97
Jan 27 '17 at 5:59
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.– CAD97
Jan 27 '17 at 5:59
add a comment |
3 Answers
3
active
oldest
votes
up vote
2
down vote
accepted
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 '17 at 7:02
coderodde
15.6k536122
1
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
add a comment |
up vote
3
down vote
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 at 1:30
Siva Srinivas
311
add a comment |
up vote
3
down vote
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 at 19:34
Pafka
312
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f153725%2fmy-implementation-of-dynamic-array%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
2
down vote
accepted
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 '17 at 7:02
coderodde
15.6k536122
1
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
add a comment |
up vote
2
down vote
accepted
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 '17 at 7:02
coderodde
15.6k536122
1
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
add a comment |
up vote
2
down vote
accepted
up vote
2
down vote
accepted
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 '17 at 7:02
coderodde
15.6k536122
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 '17 at 7:02
coderodde
15.6k536122
answered Jan 27 '17 at 7:02
coderodde
15.6k536122
answered Jan 27 '17 at 7:02
coderodde
15.6k536122
answered Jan 27 '17 at 7:02
coderodde
15.6k536122
15.6k536122
1
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
add a comment |
1
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
1
1
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.
– mtj
Jan 27 '17 at 7:46
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.
– mtj
Jan 27 '17 at 7:46
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@mtj Fair enough, yet, I am afraid, that is a matter of taste.
– coderodde
Jan 27 '17 at 7:47
@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.
– coderodde
Jan 27 '17 at 7:51
@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.
– coderodde
Jan 27 '17 at 7:51
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
@SLR You are most welcome. If you are satisfied with an answer, consider accepting it.
– coderodde
Jan 29 '17 at 19:58
add a comment |
up vote
3
down vote
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 at 1:30
Siva Srinivas
311
add a comment |
up vote
3
down vote
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 at 1:30
Siva Srinivas
311
add a comment |
up vote
3
down vote
up vote
3
down vote
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 at 1:30
Siva Srinivas
311
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 at 1:30
Siva Srinivas
311
answered Jul 23 at 1:30
Siva Srinivas
311
answered Jul 23 at 1:30
Siva Srinivas
311
answered Jul 23 at 1:30
Siva Srinivas
311
311
add a comment |
add a comment |
up vote
3
down vote
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 at 19:34
Pafka
312
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
add a comment |
up vote
3
down vote
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 at 19:34
Pafka
312
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
add a comment |
up vote
3
down vote
up vote
3
down vote
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 at 19:34
Pafka
312
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 at 19:34
Pafka
312
answered Dec 4 at 19:34
Pafka
312
answered Dec 4 at 19:34
Pafka
312
answered Dec 4 at 19:34
Pafka
312
312
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
add a comment |
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
Hi, I just wanted to congratulate you on your first review, it's valuable.
– IEatBagels
Dec 4 at 20:53
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f153725%2fmy-implementation-of-dynamic-array%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
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.– CAD97
Jan 27 '17 at 5:59