first by MerZakhar · Pull Request #1752 · mate-academy/jv-array-list

MerZakhar · 2026-02-14T20:02:40Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because several core requirements and checklist rules are still violated, which will break expected List behavior and likely cause tests to fail. The ArrayList class still has incorrect or incomplete methods: isEmpty() always returns false instead of size == 0, and remove(T value) must throw a NoSuchElementException with a clear message when the element is not found, not silently return null. There are also explicit checklist violations: you import and use java.util.Arrays (e.g., in growIfFull()) and java.util.Objects (even if unused), whereas the task forbids using Objects, Arrays, or other util helpers; instead, allocate a new Object[] with newCapacity = oldCapacity + oldCapacity / 2 and copy using System.arraycopy(...). Index validation logic is duplicated in get, set, and remove(int); per the checklist, extract a single private helper (for example checkIndexForGetSetRemove(int index)) and reuse it in all these methods to avoid repetition and keep behavior consistent. Your custom ArrayListIndexOutOfBoundsException currently ignores the passed message and always uses a hard-coded one; update its constructor to call super(message) (or build a detailed message including index and size) and ensure every thrown exception has an informative message, including the NullPointerException in addAll(...). As a minor but required style fix, remove redundant blank lines at the start of class/method bodies and simplify the add(T value) logic so it first checks if (size == elementData.length) then grows, then adds, without unnecessary else if. Once these functional and checklist-related issues are addressed, your implementation will be much closer to behaving like java.util.ArrayList, and you’re very close—fixing these points will solidify your solution.

✨ 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-02-14T20:06:20Z

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

@@ -1,24 +1,67 @@
 package core.basesyntax;

+import java.util.Arrays;


This line imports java.util.Arrays. This violates the checklist item: "Don't use Objects, Arrays, or any other util class." Remove this import and avoid using Arrays.copyOf.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

 package core.basesyntax;

+import java.util.Arrays;
+import java.util.Objects;


This line imports java.util.Objects. This violates the checklist item: "Don't use Objects, Arrays, or any other util class." Remove this import (it's unused).

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

+    private Object[] elementData;
+    private int size;
+
+    public ArrayList () {


The constructor begins after a blank line. This violates: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove redundant empty lines before the constructor.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

+    }
+
    @Override
    public void add(T value) {


The add method uses an if / else if where a simpler check would suffice (check if size == elementData.length then grow, then add). This violates: "Don't complicate if-else construction." Simplify to one conditional.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

+        if (index < 0 || index > size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to any of the methods " +
+                    "is invalid.");


Index validation is performed inline here. The checklist requires: "Don't create repeating code. If the logic of your code repeats - move it to a separate private method. For example, the index checks for the get(), set(), and remove() methods should be moved to a separate method." Extract a reusable index-validation helper and call it from all methods that require index checks.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

    @Override
    public void addAll(List<T> list) {
-
+        if (list == null) { throw new NullPointerException(); }


addAll throws new NullPointerException() without a descriptive message. This violates: "Make your exceptions informative. If you throw an Exception, you should always add an informative message so that it would be clear from the stack trace what exactly went wrong." Add a clear message.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to any of the methods " +
+                    "is invalid.");


Index validation here is duplicated inline. Extract and reuse a single index-validation helper as required by: "Don't create repeating code.... index checks for the get(), set(), and remove() methods should be moved to a separate method."

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

    }

    @Override
    public boolean isEmpty() {


isEmpty() currently returns false unconditionally. It should return size == 0 to behave like java.util.ArrayList. Implement this accordingly.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

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

+    private void growIfFull() {
+        this.elementData = Arrays.copyOf(this.elementData, this.elementData.length
+                + (this.elementData.length >> 1));
+    }


The growIfFull() method uses Arrays.copyOf(...), which violates: "Don't use Objects, Arrays, or any other util class." and also violates: "Use System.arraycopy() to move your array elements." Replace this with a manual resize: compute new capacity = old + old/2, allocate a new Object[], and use System.arraycopy(elementData, 0, newArray, 0, size) before assigning elementData = newArray. Also ensure the growth factor is 1.5x as required.

mateacademy-ai-mentor · 2026-02-14T20:06:20Z

src/main/java/core/basesyntax/ArrayListIndexOutOfBoundsException.java

 public class ArrayListIndexOutOfBoundsException extends RuntimeException {
    public ArrayListIndexOutOfBoundsException(String message) {
-        super(message);
+        super("The index passed to any of the methods is invalid.");


The constructor ignores the provided message parameter and always passes a hard-coded string to super(...). This violates the checklist: "Make your exceptions informative. If you throw an Exception, you should always add an informative message so that it would be clear from the stack trace what exactly went wrong." Change to super(message) or build a descriptive message that includes context (e.g., invalid index and size).

mateacademy-ai-mentor

Review Summary

I am requesting changes because several checklist and task requirements are currently violated, which will block this solution from being accepted. The ArrayList implementation still uses java.util.Arrays.copyOf(...) in growIfFull(), but the checklist explicitly forbids using Arrays and requires manual resizing with new Object[newCapacity] plus System.arraycopy(...) and a 1.5× growth factor. Index validation logic is duplicated across get, set, and remove(int), while the checklist requires extracting this repeated logic into a single private helper method and reusing it for consistency and maintainability. Some exceptions are not informative: addAll(List<T> list) throws new NullPointerException() with no message, and remove(T element) throws new NoSuchElementException() without a message, even though the checklist requires clear, descriptive messages for all thrown exceptions. Additionally, your custom ArrayListIndexOutOfBoundsException constructor currently ignores the message; it must call super(message) so that context like index and size is preserved when you throw it. Finally, the add(T value) method uses an unnecessary if/else branching instead of the simpler pattern if (size == elementData.length) { growIfFull(); } elementData[size++] = value;, which the checklist points out as the preferred, non-complicated structure. Once you replace Arrays.copyOf, centralize index checks, make all exception messages informative, fix the custom exception constructor, and simplify add(T), your solution will align well with the task requirements and checklist.

✨ 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-02-15T00:22:03Z

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

@@ -1,48 +1,127 @@
 package core.basesyntax;

+import java.util.Arrays;


This file imports and later uses Arrays. The checklist explicitly says: "Don't use Objects, Arrays, or any other util class." Replace Arrays.copyOf(...) with explicit allocation of a new Object[] and copy elements using System.arraycopy(...) (new capacity = oldCapacity + oldCapacity / 2). See checklist for the exact restriction.

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

+    private void growIfFull() {
+        this.elementData = Arrays.copyOf(this.elementData, this.elementData.length
+                + (this.elementData.length >> 1));


The growIfFull() method uses Arrays.copyOf(...), which violates "Don't use Objects, Arrays, or any other util class." and also does not show explicit 1.5× growth logic. The task requires the internal array to grow 1.5 times when full: "the internal array should grow 1.5 times when it is full." Implement int newCapacity = oldCapacity + oldCapacity / 2; Object[] newData = new Object[newCapacity]; System.arraycopy(elementData, 0, newData, 0, size); elementData = newData;.

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

    public void add(T value) {
-
+        if (elementData.length > size) {
+            elementData[size++] = value;
+        } else if (elementData.length == size) {
+            growIfFull();
+            elementData[size] = value;
+            this.size++;
+        }


The add(T value) method uses an unnecessary if/else branch. This violates checklist: "Don't complicate if-else construction." Simplify to: if (size == elementData.length) { growIfFull(); } elementData[size++] = value; — clearer and avoids duplicate logic.

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

    public T get(int index) {
-        return null;
+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to the get method "
+                    + "is invalid.");
+        }
+        return (T) this.elementData[index];
    }

    @Override
    public void set(T value, int index) {
+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to the set method "
+                    + "is invalid.");
+        }

+        elementData[index] = value;
    }

    @Override
    public T remove(int index) {
-        return null;
+        if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to the remove method "
+                    + "is invalid.");


Index validation logic is duplicated in get, set, and remove(int). The checklist requires: "If the logic of your code repeats - move it to a separate private method. For example, the index checks for the get(), set(), and remove() methods should be moved to a separate method." Extract a helper (e.g., checkIndex(int index)) and reuse it to keep behavior consistent and avoid repetition.

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

+        if (list == null) {
+            throw new NullPointerException();


In addAll(List<T> list) you throw new NullPointerException() without a message. The checklist requires informative exceptions: "Make your exceptions informative. If you throw an Exception, you should always add an informative message so that it would be clear from the stack trace what exactly went wrong." Add a clear message like "list must not be null".

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

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


When an element is not found remove(T element) throws new NoSuchElementException() with no message. Per checklist: "Make your exceptions informative..." include an informative message (for example: "Element not found: " + element) so failures are easier to understand.

mateacademy-ai-mentor · 2026-02-15T00:22:03Z

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

+        if (index < 0 || index > size) {
+            throw new ArrayListIndexOutOfBoundsException("The index passed to the any of add "
+                    + "method is invalid.");


You throw ArrayListIndexOutOfBoundsException with custom messages in several places. Ensure your custom exception's constructor forwards the message to RuntimeException (i.e., super(message)) so these messages are preserved. The current custom exception implementation (separate file) ignores the input message — that must be fixed. (Fix the exception constructor to public ArrayListIndexOutOfBoundsException(String message) { super(message); }.)

mateacademy-ai-mentor

Review Summary

Your solution is approved as an educational override so you can keep progressing while you refine your style and checklist habits. Functionally, your ArrayList implementation looks solid: you use a default capacity, grow the internal array by 1.5× with System.arraycopy, centralize index checks in checkIndex(), and throw informative custom and standard exceptions as required. The main remaining issue is purely stylistic: several methods (including the constructor, add(T value), addAll(List<T> list), and growIfFull()) start after an extra blank line, which violates the style rule about not beginning class or method bodies with an empty line [CHECKLIST ITEM #6].

To fix this, simply remove the empty line immediately before each of those method bodies and scan the entire ArrayList.java file for any other similar leading blank lines. This might seem minor, but consistently following these formatting rules will help you on larger projects and automated style checks. Overall, you’ve done a good job meeting the core technical requirements—now just tighten up the formatting and you’ll have a very clean 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-02-15T00:43:52Z

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

+    private static final int DEFAULT_CAPACITY = 10;
+    private Object[] elementData;
+    private int size;
+


This violates checklist item #6: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove the blank line immediately before the constructor so the constructor does not start with an empty line. See checklist for the rule .

mateacademy-ai-mentor · 2026-02-15T00:43:52Z

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

+        this.elementData = new Object[DEFAULT_CAPACITY];
+        this.size = 0;
+    }
+


This violates checklist item #6: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove the blank line immediately before the add(T value) method so the method does not start with an empty line. See checklist for the rule .

mateacademy-ai-mentor · 2026-02-15T00:43:52Z

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

+        elementData[index] = value;
+        this.size++;
    }



This violates checklist item #6: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove the blank line immediately before the addAll(List<T> list) method (and scan the rest of the file for similar redundant empty lines). See checklist for the rule .

mateacademy-ai-mentor · 2026-02-15T00:43:52Z

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

-        return false;
+        return size == 0;
+    }
+


This violates checklist item #6: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove the blank line immediately before the growIfFull() method so the method does not start with an empty line. See checklist for the rule .

first

e66bb01

mateacademy-ai-mentor suggested changes Feb 14, 2026

View reviewed changes

my-commit

2465a15

MerZakhar requested a review from mateacademy-ai-mentor February 15, 2026 00:18

mateacademy-ai-mentor suggested changes Feb 15, 2026

View reviewed changes

last

bfe409d

MerZakhar requested a review from mateacademy-ai-mentor February 15, 2026 00:41

mateacademy-ai-mentor approved these changes Feb 15, 2026

View reviewed changes

		@@ -1,24 +1,67 @@
		package core.basesyntax;

		import java.util.Arrays;

		@@ -1,48 +1,127 @@
		package core.basesyntax;

		import java.util.Arrays;

Comments

Conversation

MerZakhar commented Feb 14, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants