-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Kds #1548
base: master
Are you sure you want to change the base?
Kds #1548
Conversation
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.
It looks like there are a couple of critical issues that need to be addressed before we can approve the Pull Request. Firstly, the custom exception ArrayListIndexOutOfBoundsException
is missing, which is a core requirement of the task. Additionally, the use of the Arrays
utility class is against the checklist requirements, so you'll need to manually implement the array resizing logic. Once these issues are resolved, your implementation should be good to go! Keep up the great work, and don't hesitate to reach out if you need any help. 😊
✨ 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.
@@ -1,48 +1,105 @@ | |||
package core.basesyntax; | |||
|
|||
import java.util.Arrays; |
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.
The use of Arrays
utility class is not allowed according to the checklist. Consider implementing the array resizing logic manually without using Arrays.copyOf
.
|
||
private void checkIndex(int index) { | ||
if (index < 0 || index >= size) { | ||
throw new ArrayListIndexOutOfBoundsException("Index " + index + " is out of bounds"); |
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.
The ArrayListIndexOutOfBoundsException
is not defined in the provided files. Ensure that this custom exception is implemented as required by the task description.
|
||
private void checkIndexForAdd(int index) { | ||
if (index < 0 || index > size) { | ||
throw new ArrayListIndexOutOfBoundsException("Index " + index |
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.
The ArrayListIndexOutOfBoundsException
is not defined in the provided files. Ensure that this custom exception is implemented as required by the task description.
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Something went wrong while processing the review.
[CODE: 2]
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Great job on your implementation of the ArrayList
class! 🎉 You've made significant progress by manually implementing the array resizing logic and avoiding the use of the Arrays
utility class, which aligns with the task requirements. However, there are a few critical points to address:
-
Custom Exception: The
ArrayListIndexOutOfBoundsException
is used in your methods, but it is not defined in the codebase. It's crucial to implement this custom exception class to meet the task requirements. -
Array Resizing Logic: Well done on implementing the resizing logic manually! This is a key requirement and you've handled it correctly.
-
Use of Utility Classes: Your code adheres to the constraints by not using the
Arrays
utility class, which is excellent.
While the decision is to APPROVE your Pull Request to help you move forward with your learning, please ensure to address the missing custom exception in future updates. This will not only complete the task requirements but also enhance the robustness of your code. Keep up the good work, and continue to learn and improve! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
||
private void checkIndex(int index) { | ||
if (index < 0 || index >= size) { | ||
throw new ArrayListIndexOutOfBoundsException("Index " + index + " is out of bounds"); |
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.
The custom exception ArrayListIndexOutOfBoundsException
is used here but is not defined in the codebase. You need to implement this custom exception class as per the task requirements.
|
||
private void checkIndexForAdd(int index) { | ||
if (index < 0 || index > size) { | ||
throw new ArrayListIndexOutOfBoundsException("Index " + index |
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.
The custom exception ArrayListIndexOutOfBoundsException
is used here but is not defined in the codebase. You need to implement this custom exception class as per the task requirements.
No description provided.