solved task by plvkIvan · Pull Request #1767 · mate-academy/jv-array-list

plvkIvan · 2026-03-17T17:32:27Z

No description provided.

mateacademy-ai-mentor

Review Summary

This is a good start, but several key requirements from the task checklist need to be addressed before I can approve your solution. The most critical issue is that you are using for loops to shift array elements in methods like add(int, T), remove(int), and during resizing. A core requirement is to use the more efficient System.arraycopy() for these operations [CHECKLIST ITEM #8].

Additionally, please address the following:

The index validation logic is duplicated across several methods. This should be extracted into a private helper method to avoid repetition [CHECKLIST ITEM #5].
The growth factor 1.5 is a magic number and should be defined as a private static final constant [CHECKLIST ITEM #3].
Your NoSuchElementException needs an informative message explaining the error [CHECKLIST ITEM #10].

Once these changes are made, your implementation will be much closer to the requirements.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-17T17:34:49Z

+    }
+
+    public void resize() {
+        int newCapacity = (int) (elements.length * 1.5);


This violates checklist item #3: 'Any magic numbers should be constants'. The growth factor 1.5 should be extracted into a named constant field to improve readability, for example, private static final double GROWTH_FACTOR = 1.5;.

mateacademy-ai-mentor · 2026-03-17T17:34:49Z

+        for (int i = 0; i < elements.length; i++) {
+            newArray[i] = elements[i];
+        }


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. You should use System.arraycopy() instead of a loop. Additionally, the loop condition is incorrect as it iterates up to elements.length instead of size, which can copy unnecessary null values.

mateacademy-ai-mentor · 2026-03-17T17:34:49Z

+        for (int i = size; i > index; i--) {
+            elements[i] = elements[i - 1];
+        }


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. This for loop should be replaced with a single call to System.arraycopy() to shift the elements efficiently.

mateacademy-ai-mentor · 2026-03-17T17:34:49Z

+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("Index " + index);
+        }


This violates checklist item #5: 'Don't create repeating code'. The index validation logic is duplicated in set() and remove(int). This check should be extracted into a private helper method to avoid code repetition.

mateacademy-ai-mentor · 2026-03-17T17:34:49Z

+        for (int i = index; i < size - 1; i++) {
+            elements[i] = elements[i + 1];
+        }


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. This loop for shifting elements should be replaced with a System.arraycopy() call.

mateacademy-ai-mentor · 2026-03-17T17:34:50Z

+                for (int j = i; j < size - 1; j++) {
+                    elements[j] = elements[j + 1];
+                }


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. This loop, which shifts elements after a removal, should also be replaced with System.arraycopy().

mateacademy-ai-mentor · 2026-03-17T17:34:50Z

+                return removed;
+            }
+        }
+        throw new NoSuchElementException();


This violates checklist item #10: 'Make your exceptions informative'. When throwing NoSuchElementException, you must include an informative message that explains the cause of the error.

mateacademy-ai-mentor

Review Summary

Great job on the progress so far, especially with integrating System.arraycopy()! However, changes are needed to address code duplication. The main issue is that repeated logic hasn't been extracted into helper methods, which is a key requirement. [CHECKLIST ITEM #5]

Specifically:

The index validation check (if (index < 0 || index >= size)) is duplicated in get(), set(), and remove(int). Please move this logic into a single private helper method.
The element removal logic is present in both remove(int index) and remove(T element). To fix this, your remove(T element) method should find the element's index and then call remove(int index) to handle the actual removal.

Once you refactor to eliminate this duplicate code, your solution will be ready for approval. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-19T18:44:38Z

+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("Index " + index);
+        }


This index validation logic is repeated in the set(T, int) and remove(int) methods. This violates checklist item #5: 'Don't create repeating code.' Please extract this check into a private helper method to avoid duplication.

mateacademy-ai-mentor · 2026-03-19T18:44:38Z

+                System.arraycopy(elements, i + 1, elements, i, size - i - 1);
+
+                elements[size - 1] = null;
+                size--;


This block of code for removing an element is a duplicate of the logic in your remove(int index) method. A better approach would be to find the index of the element you want to remove and then call remove(index) to perform the actual removal. This will eliminate the repeated code, following the 'Don't Repeat Yourself' principle mentioned in checklist item #5.

mateacademy-ai-mentor

Review Summary

Great job on refactoring the index validation into a separate validateIndex method—that's a significant improvement! I am approving this submission as an educational override to help you progress, but there's one key point to remember for the future.

The logic for removing an element is currently duplicated in both remove(int index) and remove(T element). To align with the "Don't Repeat Yourself" principle, the remove(T element) method should find the index of the element and then call remove(int index) to perform the actual removal. This avoids repeating code, which is a crucial practice [CHECKLIST ITEM #5].

Keep up the excellent effort, you're very close to a perfect solution!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-19T18:53:09Z

+                final T removed = elements[i];
+
+                System.arraycopy(elements, i + 1, elements, i, size - i - 1);
+
+                elements[size - 1] = null;
+                size--;
+                return removed;


This block of code duplicates the logic from the remove(int index) method. This violates a key requirement to avoid repeating code (Checklist item #5).

Instead of duplicating the removal logic here, this method should find the index of the element (i) and then call the other remove method to do the work, like return remove(i);.

solved task

6a00fba

mateacademy-ai-mentor suggested changes Mar 17, 2026

View reviewed changes

fixed problems

b33619d

plvkIvan requested a review from mateacademy-ai-mentor March 19, 2026 18:43

mateacademy-ai-mentor suggested changes Mar 19, 2026

View reviewed changes

fixed problems

517d212

plvkIvan requested a review from mateacademy-ai-mentor March 19, 2026 18:51

mateacademy-ai-mentor approved these changes Mar 19, 2026

View reviewed changes

Conversation

plvkIvan commented Mar 17, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants