Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overridden ArrayList methods #1211

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

FkordJava
Copy link

No description provided.

Comment on lines 15 to 19
public void indexValidator(int index) {
if (index < 0 || index >= size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is not expected to be used outside this class, so make private.
don't forget to move it at the end of class, because of private.

Suggested change
public void indexValidator(int index) {
if (index < 0 || index >= size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}
}
private void indexValidator(int index) {
if (index < 0 || index >= size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}
}


public ArrayList() {
this.elementData = (T[]) new Object[DEFAULT_CAPACITY];
this.size = 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You dont need to set size=0, because java makes the same instead of you.

Suggested change
this.size = 0;

Comment on lines 21 to 29
public Object[] grow(T[] elementData, int size) {
if ((size + 1) >= elementData.length) {
int newCapacity = elementData.length + (elementData.length >> 1);
T[] newElementData = (T[]) new Object[newCapacity];
System.arraycopy(elementData, 0, newElementData, 0, size);
elementData = newElementData;
}
return elementData;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about proper access modifier for grow method.

}
}

public Object[] grow(T[] elementData, int size) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method grow should be void, because it changes the main array, e.g. this.elementData.
So, you can directly access this this.elementData.

Suggested change
public Object[] grow(T[] elementData, int size) {
public void grow(T[] elementData, int size) {

Comment on lines 40 to 42
if (index < 0 || index > size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially repeating code should be extracted into private method.

Suggested change
if (index < 0 || index > size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}
if (index < 0 || index > size) {
throw new ArrayListIndexOutOfBoundsException("Index is not valid");
}

created addIndexValidator method
created addIndexValidator method
@FkordJava FkordJava requested a review from sokimaaa November 3, 2023 16:48

private void grow(T[] elementData, int size) {
if ((size + 1) >= elementData.length) {
int newCapacity = elementData.length + (elementData.length >> 1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

binary operators reduces code readability. consider creating GROWTH_FACTOR = 1.5 constant

Suggested change
int newCapacity = elementData.length + (elementData.length >> 1);
int newCapacity = elementData.length * GROWTH_FACTOR;

Comment on lines 65 to 78
if (element == null) {
for (int i = 0; i < size; i++) {
if (elementData[i] == null) {
return remove(i);
}
}
} else {
for (int i = 0; i < size; i++) {
if (element.equals(elementData[i])) {
return remove(i);
}
}
}
throw new NoSuchElementException("Element not found");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be more simple

Suggested change
if (element == null) {
for (int i = 0; i < size; i++) {
if (elementData[i] == null) {
return remove(i);
}
}
} else {
for (int i = 0; i < size; i++) {
if (element.equals(elementData[i])) {
return remove(i);
}
}
}
throw new NoSuchElementException("Element not found");
for (int i = 0; i < size; i++) {
if (element == null ? elementData[i] == null : element.equals(elementData[i])) {
return remove(i);
}
}
throw new NoSuchElementException("Element not found");

Comment on lines 27 to 29
for (int i = size; i > index; i--) {
elementData[i] = elementData[i - 1];
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@FkordJava FkordJava requested a review from kerrrusha November 3, 2023 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants