fixed #1516
fixed #1516
Conversation
There was a problem hiding this comment.
Great job on implementing the ArrayList class! 🎉 Your code meets the task requirements and constraints effectively. While there are a few areas for improvement, such as ensuring the grow() method adjusts capacity correctly and optimizing the loop in the remove method, these are minor issues that don't critically impact functionality. Keep up the good work, and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| int newSize = size + list.size(); | ||
| if (newSize > elementData.length) { | ||
| grow(); |
There was a problem hiding this comment.
The grow() method is called only once, which might not be sufficient if the new size is significantly larger than the current capacity. Consider using a loop to ensure the capacity is sufficient.
| } | ||
|
|
||
| @Override | ||
| public T remove(T element) { | ||
| return null; | ||
| for (int i = 0; i < elementData.length; i++) { |
There was a problem hiding this comment.
The loop should iterate only up to size instead of elementData.length to avoid unnecessary checks and potential errors.
| } | ||
|
|
||
| private void grow() { | ||
| int newCapacity = (int)(size * GROW_INDEX); |
There was a problem hiding this comment.
The calculation for newCapacity should ensure that it is at least one more than the current capacity to prevent an infinite loop when size is zero.
| int newSize = size + list.size(); | ||
| if (newSize > elementData.length) { | ||
| grow(); | ||
| } |
There was a problem hiding this comment.
you can remove it because the method add() handles this logic.
| } | ||
|
|
||
| @Override | ||
| public T get(int index) { | ||
| return null; | ||
| if (index >= 0 && index < size) { |
There was a problem hiding this comment.
You can move the reverse condition throw an exception in a private method and reuse that method.
| if (index >= 0 && index < size) { | ||
| elementData[index] = value; | ||
| } else { | ||
| throw new ArrayListIndexOutOfBoundsException("Index: " + index + ", is invalid "); | ||
| } |
There was a problem hiding this comment.
avoid using if-else construction.
| if (index >= 0 && index < size) { | |
| elementData[index] = value; | |
| } else { | |
| throw new ArrayListIndexOutOfBoundsException("Index: " + index + ", is invalid "); | |
| } | |
| checkIndex(index); | |
| elementData[index] = value; |
And create private method:
private void checkIndex(int index) {
if (index < 0 || index >= size) {
throw new ArrayListIndexOutOfBoundsException("Index: " + index + ", is invalid ");
}
}
| } | ||
|
|
||
| @Override | ||
| public T remove(int index) { | ||
| return null; | ||
| if (index >= 0 && index < size) { |
There was a problem hiding this comment.
the same. just call checkIndex method
There was a problem hiding this comment.
Use here method that you have already
| if (elementData[i] == null && element == null) { | ||
| return remove(i); | ||
| } | ||
| if (elementData[i] != null && elementData[i].equals(element)) { | ||
| return remove(i); | ||
| } |
There was a problem hiding this comment.
can you combine these conditionals?
| } | ||
|
|
||
| @Override | ||
| public T remove(int index) { | ||
| return null; | ||
| if (index >= 0 && index < size) { |
| if ((elementData[i] == null && element == null) | ||
| || (elementData[i] != null && elementData[i].equals(element))) { |
There was a problem hiding this comment.
| if ((elementData[i] == null && element == null) | |
| || (elementData[i] != null && elementData[i].equals(element))) { | |
| if (elementData[i] == element | |
| || elementData[i] != null && elementData[i].equals(element)) { |
| } | ||
|
|
||
| @Override | ||
| public T remove(int index) { | ||
| return null; | ||
| if (index >= 0 && index < size) { |
There was a problem hiding this comment.
Use here method that you have already
| elementData[--size] = null; | ||
| return deleteValue; | ||
| } | ||
| throw new ArrayListIndexOutOfBoundsException("Invalid index: " + index); |
There was a problem hiding this comment.
| throw new ArrayListIndexOutOfBoundsException("Invalid index: " + index); |
| if (index < 0 || index > size) { | ||
| throw new ArrayListIndexOutOfBoundsException("Invalid index: " + index); | ||
| } |
There was a problem hiding this comment.
you can extract this check to private method
| if (size == elementData.length) { | ||
| grow(); | ||
| } |
There was a problem hiding this comment.
| if (size == elementData.length) { | |
| grow(); | |
| } | |
| grow(); |
There was a problem hiding this comment.
You can add this inside method grow
No description provided.