-
Notifications
You must be signed in to change notification settings - Fork 1.6k
f #1780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
f #1780
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,48 +1,113 @@ | ||
| package core.basesyntax; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.NoSuchElementException; | ||
| import java.util.Objects; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import (java.util.Objects) violates the checklist: "Don't use Objects, Arrays, or any other util class." Do not use Objects.equals(); implement equality checks manually (handle nulls explicitly). |
||
|
|
||
| public class ArrayList<T> implements List<T> { | ||
| private static final int START_INDEX = 0; | ||
| private static final int DEFAULT_CAPACITY = 10; | ||
| private int capacity; | ||
| private T[] elementData; | ||
| private int size; | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no resize/grow helper method. The checklist explicitly requires: "Resize the array in a separate method." Add a private method that grows |
||
| public ArrayList() { | ||
| elementData = (T[]) new Object[DEFAULT_CAPACITY]; | ||
| capacity = DEFAULT_CAPACITY; | ||
| } | ||
|
|
||
| @Override | ||
| public void add(T value) { | ||
|
|
||
| if (size >= capacity) { | ||
| grow(); | ||
| } | ||
| elementData[size] = value; | ||
| size++; | ||
| } | ||
|
|
||
| @Override | ||
| public void add(T value, int index) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method signature here is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The indexed add method uses the signature |
||
| if (index > size || index < START_INDEX) { | ||
| throw new ArrayListIndexOutOfBoundsException("index %s out of bounds for length %s" | ||
| .formatted(index, size)); | ||
| } | ||
|
Comment on lines
+28
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You perform index validation here manually instead of reusing the index-check helper. The requirements state you should move repeated index checks to a private helper and use it for
Comment on lines
+28
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block performs its own index validation instead of reusing the shared index-check helper. The requirements state: "Index checking logic that is shared by methods like |
||
| if (size >= capacity) { | ||
| grow(); | ||
| } | ||
| System.arraycopy(elementData, index, elementData, index + 1, size - index); | ||
| elementData[index] = value; | ||
| size++; | ||
|
|
||
| } | ||
|
|
||
| @Override | ||
| public void addAll(List<T> list) { | ||
|
|
||
| for (int i = 0; i < list.size(); i++) { | ||
| add(list.get(i)); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public T get(int index) { | ||
| return null; | ||
| checkIndex(index); | ||
| return elementData[index]; | ||
| } | ||
|
|
||
| @Override | ||
| public void set(T value, int index) { | ||
| checkIndex(index); | ||
|
|
||
| elementData[index] = value; | ||
| } | ||
|
|
||
| @Override | ||
| public T remove(int index) { | ||
| return null; | ||
| checkIndex(index); | ||
| T value = elementData[index]; | ||
| System.arraycopy(elementData, index + 1, elementData, index, size - index - 1); | ||
| size--; | ||
|
Comment on lines
+65
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After shifting elements and decrementing size you must null out the former last element position to avoid memory leaks and ensure toString()/iteration don't see stale data. The requirements say: "When an element is removed, shift |
||
| return value; | ||
| } | ||
|
|
||
| @Override | ||
| public T remove(T element) { | ||
| return null; | ||
| int index = 0; | ||
| for (T elementDatum : elementData) { | ||
| if (Objects.equals(elementDatum, element)) { | ||
| T value = elementData[index]; | ||
| System.arraycopy(elementData, index + 1, elementData, index, size - index); | ||
| size--; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The loop iterates over the whole backing array ( |
||
| return value; | ||
| } | ||
| index++; | ||
| } | ||
| throw new NoSuchElementException(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throwing |
||
| } | ||
|
|
||
| @Override | ||
| public int size() { | ||
| return 0; | ||
| return size; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| @Override | ||
| public boolean isEmpty() { | ||
| return false; | ||
| return size == 0; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return Arrays.toString(elementData); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. toString() uses |
||
| } | ||
|
|
||
| private void grow() { | ||
| capacity += capacity / 2; | ||
| elementData = Arrays.copyOf(elementData, capacity); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The grow() method uses |
||
| } | ||
|
|
||
| private void checkIndex(int index) { | ||
| if (index > size - 1 || index < START_INDEX) { | ||
| throw new ArrayListIndexOutOfBoundsException("index %s out of bounds for length %s" | ||
| .formatted(index, size)); | ||
| } | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package core.basesyntax; | ||
|
|
||
| public class Main { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The internal storage There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import violates the checklist: "Don't use Objects, Arrays, or any other util class." You also use Arrays.copyOf and Arrays.toString below; remove this import and avoid java.util.Arrays usage in favor of allocating a new backing array and using System.arraycopy (and building toString using elements up to size). |
||
| public static void main(String[] args) { | ||
| ArrayList<String> test = new ArrayList<>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import violates the checklist: "Don't use Objects, Arrays, or any other util class." Replace Objects.equals usage with an explicit null-safe comparison (e.g. |
||
| System.out.println(test); | ||
| for (int i = 0; i < 5; i++) { | ||
| test.add(Integer.toString(i)); | ||
| } | ||
| System.out.println(test); | ||
| test.remove("2"); | ||
| System.out.println(test); | ||
|
|
||
| java.util.ArrayList<String> utilTest = new java.util.ArrayList<>(); | ||
| for (int i = 0; i < 5; i++) { | ||
| utilTest.add(Integer.toString(i)); | ||
| } | ||
| System.out.println(utilTest); | ||
| utilTest.remove(-2); | ||
| System.out.println(utilTest); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import (java.util.Arrays) violates the checklist: "Don't use Objects, Arrays, or any other util class." Remove this import and avoid using Arrays.* methods (use manual logic + System.arraycopy where required).