Done own realisation of ArrayList by ZarichniyYaroslav · Pull Request #1739 · mate-academy/jv-array-list

ZarichniyYaroslav · 2026-02-03T11:40:07Z

No description provided.

mateacademy-ai-mentor

Review Summary

Your implementation of ArrayList is a good start, but I'm requesting changes because several core requirements have not been met. Please address the following points:

Array Growth Logic: The array must grow 1.5 times its current capacity (array.length) when full. Your current logic incorrectly triples the size (size). The growth condition should also be size == array.length.
Use of System.arraycopy(): The task explicitly requires using System.arraycopy() for moving and copying elements. Your implementation uses manual for loops in add(T, int), remove(int), and the resizing method. These loops must be replaced. [CHECKLIST ITEM #8]
Prohibited Utility Class: The use of java.util.Objects is not allowed by the requirements. Please remove the import and implement a manual null-safe check in the remove(T element) method. [CHECKLIST ITEM #2]
Duplicated Code: The index validation logic is repeated in set() and remove(int). This should be extracted into a private helper method to adhere to the requirements. [CHECKLIST ITEM #5]

Once these issues are resolved, your solution will be much closer to the requirements. Keep up the effort!

✨ 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-03T11:41:55Z

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

 package core.basesyntax;

+import java.util.NoSuchElementException;
+import java.util.Objects;


Using java.util.Objects is not allowed according to checklist item #2. Please remove this import and implement the null-safe equality check in the remove(T element) method manually.

mateacademy-ai-mentor · 2026-02-03T11:41:55Z

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

+        if (index > size || index < 0) {
+            throw new ArrayListIndexOutOfBoundsException("ArrayList index out of bounds");
+        }
+        if (size == array.length - 1) {


The condition to grow the array should be when it's completely full, i.e., size == array.length. This condition triggers the growth prematurely when there is still one slot available.

mateacademy-ai-mentor · 2026-02-03T11:41:55Z

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

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


Checklist item #8 requires using System.arraycopy() to move array elements. This manual loop should be replaced.

mateacademy-ai-mentor · 2026-02-03T11:41:56Z

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

+        if (index >= size || index < 0) {
+            throw new ArrayListIndexOutOfBoundsException("ArrayList index out of bounds");
+        }


This index validation logic is repeated in set() and remove(int). Checklist item #5 requires moving repeated logic to a separate private method. A helper method like private void checkIndex(int index) would be a good solution.

mateacademy-ai-mentor · 2026-02-03T11:41:56Z

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

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


This loop for shifting elements should be replaced by a call to System.arraycopy() to comply with checklist item #8.

mateacademy-ai-mentor · 2026-02-03T11:41:56Z

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

    public T remove(T element) {
-        return null;
+        for (int i = 0; i < size; i++) {
+            if (Objects.equals(array[i], element)) {


This violates checklist item #2, which prohibits using util classes like Objects. The comparison logic needs to handle null elements without using this helper method.

mateacademy-ai-mentor · 2026-02-03T11:41:56Z

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

+    }
+
+    private Object[] grow() {
+        Object[] growedArray = new Object[size + (size << 1)];


The new array size calculation is incorrect. The requirement is to grow 1.5 times the current capacity. The current logic size + (size << 1) results in 3 times the current size, not 1.5 times the array.length (capacity). The formula should be similar to (int) (array.length * 1.5).

mateacademy-ai-mentor · 2026-02-03T11:41:56Z

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

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


Checklist item #8 requires using System.arraycopy() to copy elements from the old array to the new, more capacious one.

…idnt use Objects.equals , used System.arraycopy

mateacademy-ai-mentor

Review Summary

Great job addressing most of the feedback from the previous review! You're very close to completing this task. However, there are a few critical issues with the array growth logic that need to be fixed before this can be approved.

Here are the required changes:

Incorrect Growth Factor: The grow() method currently triples the array's capacity (array.length + (array.length << 1)). The requirement is to increase it by 1.5 times. You can achieve this with a right bit shift, for example: oldCapacity + (oldCapacity >> 1).
Incorrect Growth Condition: In the add(T value, int index) method, the condition if (size == array.length - 1) causes the array to resize too early. The array should only grow when it's completely full (size == array.length).
Duplicated Code: The get(int index) method contains an index check that should be replaced with a call to your indexValidation() helper method to avoid code duplication.

Please address these points, and I'll be happy to review your code again!

✨ 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-03T12:02:16Z

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

+        if (index > size || index < 0) {
+            throw new ArrayListIndexOutOfBoundsException("ArrayList index out of bounds");
+        }
+        if (size == array.length - 1) {


The condition to grow the array is incorrect. The array should grow when it is full (size == array.length), but this condition size == array.length - 1 triggers the growth when there is still one empty slot left.

mateacademy-ai-mentor · 2026-02-03T12:02:16Z

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

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


This violates checklist item #5: 'Don't create repeating code.' You have already created a helper method indexValidation(index) for this exact check. Please reuse it here to avoid code duplication.

mateacademy-ai-mentor · 2026-02-03T12:02:16Z

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

+    }
+
+    private Object[] grow() {
+        Object[] growedArray = new Object[array.length + (array.length << 1)];


The requirement is for the array to grow 1.5 times its current capacity. The current logic, array.length + (array.length << 1), is equivalent to array.length * 3, which triples the capacity.

To achieve 1.5x growth, you could use an expression like array.length + (array.length >> 1), which adds half of the current length to itself.

mateacademy-ai-mentor

Review Summary

Great job on implementing the ArrayList! You've successfully addressed the major feedback from the last review, and your code is functionally correct. I am approving your solution to help you progress, but please take a look at these final suggestions to fully align your code with java.util.ArrayList's behavior:

Growth Condition: For consistency, it's better to use size == array.length as the condition for resizing in all add methods.
Memory Management: In your remove methods, consider setting the now-unused slot to null after shifting elements (e.g., array[size] = null;). This is a good practice to prevent potential memory leaks.
Readability: The logic for finding an element in remove(T element) can be made clearer by separating the checks for null and non-null elements.

You're doing great work. Keep it up!

✨ 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-03T12:18:52Z

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

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


While this condition is not incorrect, it's slightly inconsistent with the one in add(T value, int index), which uses size == array.length. To maintain consistency and more accurately represent the condition for resizing (when the array is exactly full), it's better to use size == array.length here as well.

mateacademy-ai-mentor · 2026-02-03T12:18:52Z

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

+        }
+        T removed = (T) array[index];
+        System.arraycopy(array, index + 1, array, index, size - index - 1);
+        size--;


To fully align with java.util.ArrayList's behavior and prevent potential memory leaks, it's a good practice to set the now-unused slot to null. This allows the garbage collector to reclaim the memory if the removed object is no longer referenced elsewhere.

You can add array[size] = null; after this line.

mateacademy-ai-mentor · 2026-02-03T12:18:52Z

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

    public T remove(T element) {
-        return null;
+        for (int i = 0; i < size; i++) {
+            if (array[i] == element || (array[i] != null && array[i].equals(element))) {


This logic for finding the element is correct, but it's a bit complex to read at a glance. To improve clarity and align more closely with java.util.ArrayList's implementation, consider separating the logic for null and non-null elements.

A common and more readable approach is:

if (element == null) { for (int i = 0; i < size; i++) { if (array[i] == null) { return remove(i); } } } else { for (int i = 0; i < size; i++) { if (element.equals(array[i])) { return remove(i); } } }

Elena-Bruyako

Good job! See comments

Elena-Bruyako · 2026-02-04T08:07:38Z

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

+    private static final int NEW_ARRAY_LIST_SIZE = 0;
+    private Object[] array;
+    private int size;
+
+    public ArrayList() {
+        array = new Object[NEW_ARRAY_SIZE];
+        size = NEW_ARRAY_LIST_SIZE;
+    }


int type = 0 by default

Suggested change

private static final int NEW_ARRAY_LIST_SIZE = 0;

private Object[] array;

private int size;

public ArrayList() {

array = new Object[NEW_ARRAY_SIZE];

size = NEW_ARRAY_LIST_SIZE;

}

private Object[] array;

private int size;

public ArrayList() {

array = new Object[NEW_ARRAY_SIZE];

}

Elena-Bruyako · 2026-02-04T08:08:40Z

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

    public T get(int index) {
-        return null;
+        if (indexValidation(index)) {
+            throw new ArrayListIndexOutOfBoundsException("ArrayList index out of bounds");


replace throwing exception to indexValidation method

Elena-Bruyako · 2026-02-04T08:08:47Z

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

+        return growedArray;
+    }
+
+    private boolean indexValidation(int index) {


Suggested change

private boolean indexValidation(int index) {

private void indexValidation(int index) {

Elena-Bruyako · 2026-02-04T08:09:00Z

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

    public T remove(T element) {
-        return null;
+        for (int i = 0; i < size; i++) {
+            if (array[i] == element || (array[i] != null && array[i].equals(element))) {


Suggested change

if (array[i] == element || (array[i] != null && array[i].equals(element))) {

if (array[i] == element || array[i] != null && array[i].equals(element)) {

Done own realisation of ArrayList

1f07a43

mateacademy-ai-mentor suggested changes Feb 3, 2026

View reviewed changes

Changed grow implementation , added new help method indexValidation,d…

6a1cb2b

…idnt use Objects.equals , used System.arraycopy

ZarichniyYaroslav requested a review from mateacademy-ai-mentor February 3, 2026 12:00

mateacademy-ai-mentor suggested changes Feb 3, 2026

View reviewed changes

ZarichniyYaroslav added 2 commits February 3, 2026 13:05

Changed grow method, used helping method indexValidation

9e5407d

done

be64f55

ZarichniyYaroslav requested a review from mateacademy-ai-mentor February 3, 2026 12:16

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

View reviewed changes

Elena-Bruyako suggested changes Feb 4, 2026

View reviewed changes

	private boolean indexValidation(int index) {
	private void indexValidation(int index) {

	if (array[i] == element \|\| (array[i] != null && array[i].equals(element))) {
	if (array[i] == element \|\| array[i] != null && array[i].equals(element)) {

Comments

Conversation

ZarichniyYaroslav commented Feb 3, 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!

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!

Elena-Bruyako left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants