Implement ArrayList logic#1790
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has several critical violations that must be fixed:
-
Uses
java.util.Objects- The checklist explicitly prohibits usingObjects,Arrays, or any other util classes. You must replaceObjects.equals()with a manual null-safe comparison. -
remove(T element)returnsnullinstead of throwingNoSuchElementException- When the element is not found, the method should throwNoSuchElementExceptionwith an informative message, not returnnull. -
Empty lines before class and method implementations - Multiple empty lines before constructors and methods violate the formatting rule "Don't begin class or method implementation with an empty line."
-
Modifier order violation - The
static finalon line 6 should befinal staticto comply with JLS modifier order.
Please fix these issues and resubmit. Focus on removing the Objects import and implementing proper exception throwing for the remove method.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -1,48 +1,101 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| import java.util.Objects; | |||
There was a problem hiding this comment.
This imports java.util.Objects which is explicitly prohibited by the checklist: 'Don't use Objects, Arrays, or any other util class.'
| return null; | ||
| int index = -1; | ||
| for (int i = 0; i < size; i++) { | ||
| if (Objects.equals(data[i], element)) { |
There was a problem hiding this comment.
Uses Objects.equals() which violates the checklist prohibition on using Objects class.
| } | ||
| return index == -1 ? null : remove(index); |
There was a problem hiding this comment.
The method returns null when element is not found, but the task requires throwing NoSuchElementException in this case. The return type should be changed from T to T with exception thrown: throw new NoSuchElementException("Element not found: " + element).
|
|
||
| public class ArrayList<T> implements List<T> { |
There was a problem hiding this comment.
Empty line before class implementation - violates checklist: 'Don't begin class or method implementation with an empty line.'
|
|
||
| public ArrayList() { |
There was a problem hiding this comment.
Empty line before constructor - violates checklist rule about empty lines before implementations.
|
|
||
| @Override | ||
| public T remove(T element) { |
There was a problem hiding this comment.
Empty line before method - violates checklist rule.
|
|
||
| @Override | ||
| public int size() { |
There was a problem hiding this comment.
Empty line before method - violates checklist rule.
|
|
||
| @Override | ||
| public boolean isEmpty() { |
There was a problem hiding this comment.
Empty line before method - violates checklist rule.
|
|
||
| private void grow() { |
There was a problem hiding this comment.
Empty line before private method - violates checklist rule.
|
|
||
| private void checkIndex(int index) { |
There was a problem hiding this comment.
Empty line before private method - violates checklist rule.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has two critical checklist violations that must be addressed:
1. Still using java.util.Objects class (CHECKLIST VIOLATION)
- The import
java.util.Objectsand usage ofObjects.equals(data[i], element)on line 71 directly violates the checklist requirement: "Don't use Objects, Arrays, or any other util class." - Replace with a manual null-safe comparison, for example:
(element == null) ? (data[i] == null) : element.equals(data[i])2. NoSuchElementException lacks informative message
- The exception on line 76 should include the element that wasn't found, e.g.
throw new NoSuchElementException("Element not found: " + element);These are explicit checklist requirements that must be met. Once these issues are resolved, your implementation should be ready for approval.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| package core.basesyntax; | ||
|
|
||
| import java.util.NoSuchElementException; | ||
| import java.util.Objects; |
There was a problem hiding this comment.
The checklist explicitly states 'Don't use Objects, Arrays, or any other util class.' Remove this import and replace Objects.equals() with a manual null-safe comparison. For example: (data[i] == null) ? (element == null) : data[i].equals(element)
| return null; | ||
| int index = -1; | ||
| for (int i = 0; i < size; i++) { | ||
| if (Objects.equals(data[i], element)) { |
There was a problem hiding this comment.
Replace with manual null-safe comparison instead of Objects.equals()
| break; | ||
| } | ||
| } | ||
| if (index == -1) { |
There was a problem hiding this comment.
The exception should have an informative message as required by the checklist. Include the element that was not found, e.g.: throw new NoSuchElementException("Element not found: " + element);
|
|
||
| import java.util.NoSuchElementException; | ||
| import java.util.Objects; | ||
|
|
There was a problem hiding this comment.
Remove the empty line before the class declaration. The checklist states 'Don't begin class or method implementation with an empty line.'
| private int currentCapacity; | ||
| private Object[] data; | ||
| private int size; | ||
|
|
There was a problem hiding this comment.
Remove the empty line before the constructor. The checklist states 'Don't begin class or method implementation with an empty line.'
| throw new NoSuchElementException(); | ||
| } | ||
| return remove(index); | ||
| } |
There was a problem hiding this comment.
Remove the empty line before this method. The checklist states 'Don't begin class or method implementation with an empty line.'
| return 0; | ||
| return size; | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove the empty line before this method. The checklist states 'Don't begin class or method implementation with an empty line.'
| public boolean isEmpty() { | ||
| return false; | ||
| return size == 0; | ||
| } |
There was a problem hiding this comment.
Remove the empty line before this method. The checklist states 'Don't begin class or method implementation with an empty line.'
| currentCapacity = newCapacity; | ||
| data = newData; | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove the empty line before this method. The checklist states 'Don't begin class or method implementation with an empty line.'
| private void checkIndex(int index) { | ||
| if (index < 0 || index > size - 1) { | ||
| throw new ArrayListIndexOutOfBoundsException("Index: " + index + ", size: " + size); | ||
| } |
There was a problem hiding this comment.
Remove the empty line before this method. The checklist states 'Don't begin class or method implementation with an empty line.'
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your ArrayList implementation has successfully resolved all previous issues and meets every requirement. The custom compareElements method properly replaces the prohibited java.util.Objects usage, and the NoSuchElementException now includes an informative message. Your code demonstrates excellent understanding of single responsibility principles with well-separated grow(), checkIndex(), and compareElements() methods. The 1.5x capacity growth using bitwise shift, custom exception handling with messages, and proper use of System.arraycopy() all show solid Java fundamentals. Great work on extracting magic numbers to constants as well — this makes the code maintainable and professional.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.