Solved task by neuklearna · Pull Request #1734 · mate-academy/jv-array-list

neuklearna · 2026-01-25T00:14:25Z

No description provided.

mateacademy-ai-mentor

Review Summary

Your implementation of ArrayList is a solid effort and correctly covers the basic functionality. However, several key requirements from the task description need to be addressed before this can be approved.

Here are the main points to fix:

Array Manipulation: The task explicitly requires using System.arraycopy() for moving elements. Please replace the manual for loops in your add(T element, int index) and remove(int index) methods with System.arraycopy(). [CHECKLIST ITEM #8]
Magic Numbers: The growth factor 1.5 and the default capacity 10 should be declared as private static final constants to improve readability and maintainability. [CHECKLIST ITEM #3]
Code Duplication: The index validation logic is repeated in get(), set(), and remove(int). Please extract this into a single private helper method to follow the DRY principle. [CHECKLIST ITEM #5]
Variable Naming: The internal array field named array is too abstract. A more descriptive name like elements or elementData should be used. [CHECKLIST ITEM #6]

Please make these changes and resubmit. You're on the right track!

✨ 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-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

+import java.util.NoSuchElementException;

 public class ArrayList<T> implements List<T> {
+    int DEFAULT_CAPACITY = 10;


This violates checklist item #3: 'Any magic numbers should be constants'. This field should be declared as a private static final constant to adhere to Java conventions for constants.

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

+    int DEFAULT_CAPACITY = 10;
+    int size = 0;
+
+    Object[] array = new Object[DEFAULT_CAPACITY];


This violates checklist item #6: 'Do not use abstract words like string or array as variable names'. Consider a more descriptive name like elementData or elements.

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

+    }

+    public void grow() {
+        int newCapacity = (int) (array.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 to improve readability and maintainability.

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

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


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

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

    public void add(T value, int index) {
-
+        if (index < 0 || index > size) {
+            throw new ArrayListIndexOutOfBoundsException("Index out of bounds");


This violates checklist item #10: 'Make your exceptions informative'. This message is not as helpful as it could be. Please include the invalid index and the current size of the list, similar to how you've done in other methods like get().

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

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


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. Using System.arraycopy() is more efficient for shifting elements than a manual loop.

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

    @Override
    public T get(int index) {
-        return null;
+        if (index < 0 || index >= size) {


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

mateacademy-ai-mentor · 2026-01-25T00:16:14Z

src/main/java/core/basesyntax/ArrayList.java

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


This violates checklist item #8: 'Use System.arraycopy() to move your array elements'. A call to System.arraycopy() should be used here to shift the elements after removal.

mateacademy-ai-mentor

Review Summary

Good progress on this task, and thank you for addressing some of the previous feedback. However, changes are needed because several critical requirements from the task description, which were also mentioned in the last review, remain unaddressed. Please focus on these three key points:

Use System.arraycopy(): The task requires you to use System.arraycopy() for all array manipulation. The manual for loops in your add(T, int), remove(int), and grow() methods must be replaced. [CHECKLIST ITEM #8]
Eliminate Magic Numbers: The growth factor 1.5 is still hardcoded. Please declare it as a private static final constant. [CHECKLIST ITEM #3]
Improve Variable Naming: The internal array field named array is too generic. It should be renamed to something more descriptive, like elementData or elements. [CHECKLIST ITEM #6]

You are very close to a complete solution! Please make these adjustments.

✨ 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-01-25T00:38:04Z

src/main/java/core/basesyntax/ArrayList.java

 public class ArrayList<T> implements List<T> {
+    private static final int DEFAULT_CAPACITY = 10;
+    private int size = 0;
+    private Object[] array = new Object[DEFAULT_CAPACITY];


This was mentioned in the previous review. According to checklist item #6, the internal array should have a more descriptive name like elementData or elements instead of the generic array.

mateacademy-ai-mentor · 2026-01-25T00:38:04Z

src/main/java/core/basesyntax/ArrayList.java

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


This was a key point in the previous review. The task requires using System.arraycopy() to shift elements instead of a manual for loop. This violates checklist item #8.

mateacademy-ai-mentor · 2026-01-25T00:38:04Z

src/main/java/core/basesyntax/ArrayList.java

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


Similar to the add method, this for loop should be replaced with a call to System.arraycopy() as required by checklist item #8. This was also mentioned in the previous review.

mateacademy-ai-mentor · 2026-01-25T00:38:04Z

src/main/java/core/basesyntax/ArrayList.java

+    }
+
+    private void grow() {
+        int newCapacity = (int) (array.length * 1.5);


The growth factor 1.5 is a magic number. Please declare it as a private static final constant, as required by checklist item #3 and mentioned in the previous review.

mateacademy-ai-mentor · 2026-01-25T00:38:04Z

src/main/java/core/basesyntax/ArrayList.java

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


This loop for copying elements to the new array should also be replaced with System.arraycopy() to adhere to checklist item #8.

mateacademy-ai-mentor

Review Summary

Great job addressing the previous feedback! You've done excellent work implementing System.arraycopy and improving the code's readability. I am approving your solution so you can proceed, but I want to highlight a couple of areas for future improvement.

To make your code even better, consider extracting the capacity check from both add methods into a single private helper method to avoid code duplication [CHECKLIST ITEM #5]. Also, try to make the exception message in your add(T value, int index) method more informative by including the invalid index and the list's size, which is a great practice for debugging [CHECKLIST ITEM #10].

Keep up the fantastic work!

✨ 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-01-25T00:48:11Z

src/main/java/core/basesyntax/ArrayList.java

    @Override
    public void add(T value) {
-
+        if (size == elementData.length) {


This capacity check is repeated in the add(T value, int index) method (line 25). This violates checklist item #5: 'Don't create repeating code.' Consider moving this logic into a separate private helper method, for example, ensureCapacity(), and calling it from both add methods.

mateacademy-ai-mentor · 2026-01-25T00:48:12Z

src/main/java/core/basesyntax/ArrayList.java

    public void add(T value, int index) {
-
+        if (index < 0 || index > size) {
+            throw new ArrayListIndexOutOfBoundsException("Index out of bounds");


This exception message is not very informative. Checklist item #10 requires informative exception messages. It would be more helpful to include the invalid index and the current size, similar to the message in your checkIndex method.

Solved task

f49b025

mateacademy-ai-mentor suggested changes Jan 25, 2026

View reviewed changes

neuklearna added 4 commits January 25, 2026 01:18

Fixed checkstyle errors

8dd6883

Removed comments and fixed formatting

649aeb9

Fixed checkstyle with estra spaces

b19ea80

Added extra space at the end of project

2aabdcc

neuklearna requested a review from mateacademy-ai-mentor January 25, 2026 00:36

mateacademy-ai-mentor suggested changes Jan 25, 2026

View reviewed changes

Replaced magic number on final double and used arraycopy

e5eed3f

neuklearna requested a review from mateacademy-ai-mentor January 25, 2026 00:46

mateacademy-ai-mentor approved these changes Jan 25, 2026

View reviewed changes

Comments

Conversation

neuklearna commented Jan 25, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants