https://github.com/DmytroBorysenko2004/jv-array-list #1515
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this implementation! While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Abstract Class Declaration: The
ArrayListclass is currently declared as abstract, which prevents it from being instantiated. Since it implements all methods from theListinterface, consider removing theabstractkeyword. -
addMethod with Index: The current implementation does not allow adding elements at the end of the list (i.e., at indexsize). Please modify therangeCheckmethod or add a condition to allow this. -
remove(Object element)Method: This method should not throw aNoSuchElementExceptionif the element is not found. Instead, it should returnfalse, which is the standard behavior for aList. -
addAllMethod: Ensure that the method checks if the input list is null to avoid aNullPointerException.
Once these issues are resolved, your implementation will be much stronger and ready for approval. Keep up the great work, and I look forward to seeing your improvements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
| import java.util.List; | ||
| import java.util.NoSuchElementException; | ||
|
|
||
| public abstract class ArrayList<T> implements List<T> { |
There was a problem hiding this comment.
The class ArrayList is declared as abstract, but it implements all methods from the List interface. Consider removing the abstract keyword to allow instantiation of this class.
| } | ||
|
|
||
| public void add(T value, int index) { | ||
| rangeCheck(index); |
There was a problem hiding this comment.
The rangeCheck method is used here, which will throw an exception if the index is equal to the size. However, adding at the end of the list (i.e., at index size) should be allowed. Consider modifying the rangeCheck method or adding a condition to allow this.
| remove(index); | ||
| return true; | ||
| } else { | ||
| throw new NoSuchElementException("Element not found"); |
There was a problem hiding this comment.
The remove(Object element) method should not throw a NoSuchElementException if the element is not found. Instead, it should return false, as this is the typical behavior for a List implementation.
|
|
||
| public void addAll(List<T> list) { |
There was a problem hiding this comment.
The addAll method should check if the input list is null to avoid a NullPointerException. Consider adding a null check before proceeding with the method logic.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Thank you for your hard work on the ArrayList implementation! You've made significant progress, but there are still a few critical areas that need attention to fully meet the task requirements:
-
Unimplemented Methods: Several methods such as
set,addAll,contains,iterator, andtoArrayare not fully implemented. These methods are crucial for the functionality of theArrayListand should be completed to align with theListinterface requirements. -
addMethod with Index: Theadd(T value, int index)method currently does not allow adding elements at the end of the list. Consider modifying therangeCheckmethod or adding a condition to permit this. -
addAllMethod: Ensure to add a null check for the input list to prevent aNullPointerException.
Despite these issues, the decision is to APPROVE your Pull Request to help you move forward with your learning journey. Please review the comments and address these critical issues in future iterations to enhance your code's functionality and robustness. Keep up the great work and continue improving your skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| public void add(T value, int index) { | ||
| rangeCheck(index); |
There was a problem hiding this comment.
The rangeCheck method is used here, which does not allow adding elements at the end of the list (i.e., at index size). Consider modifying the rangeCheck method or adding a condition to allow adding at the end.
| size++; | ||
| } | ||
|
|
||
| public void addAll(List<T> list) { |
There was a problem hiding this comment.
The addAll method should check if the input list is null to avoid a NullPointerException. Consider adding a null check before proceeding with the operation.
| public T set(int index, T element) { | ||
| return null; |
There was a problem hiding this comment.
The set method is not implemented and returns null. This method should update the element at the specified index and return the old element.
| public boolean addAll(Collection<? extends T> c) { | ||
| return false; |
There was a problem hiding this comment.
The addAll method is not implemented and returns false. This method should add all elements from the specified collection to the list.
| public boolean contains(Object o) { | ||
| return false; |
There was a problem hiding this comment.
The contains method is not implemented and returns false. This method should check if the list contains the specified element.
| public Iterator<T> iterator() { | ||
| return null; |
There was a problem hiding this comment.
The iterator method is not implemented and returns null. This method should return an iterator over the elements in the list.
| public Object[] toArray() { | ||
| return new Object[0]; |
There was a problem hiding this comment.
The toArray method is not implemented and returns an empty array. This method should return an array containing all elements in the list.
No description provided.