Ready for push, passed tests and checkstyle#1920
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has two critical violations of the checklist requirements that must be fixed before approval. First, [CHECKLIST ITEM #6] explicitly prohibits using java.util.Objects class, but your code uses Objects.equals() at line 21 — replace this with direct null-safe comparison: key == null ? otherKey == null : key.equals(otherKey). Second, [CHECKLIST ITEM #5] requires avoiding repeating code patterns, yet the key-search loop logic is duplicated in both put() and get() methods — extract this into a private helper method like findKeyIndex(K key) that returns the index or -1 if not found. These are functional requirements from the checklist, not style preferences. Once you fix these two violations, your implementation should meet all requirements.
✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.
Footnotes
| package core.basesyntax.impl; | ||
|
|
||
| import core.basesyntax.Storage; | ||
| import java.util.Objects; |
There was a problem hiding this comment.
Checklist item #6 states 'Don't use class Objects'. Replace Objects.equals() with direct null-safe comparison or use .equals() method on the key objects.
| @Override | ||
| public void put(K key, V value) { | ||
| for (int i = 0; i < size; i++) { | ||
| if (Objects.equals(this.key[i], key)) { |
There was a problem hiding this comment.
Checklist item #5 states 'Don't create repeating code'. The key comparison logic in put() and get() methods follows the same pattern. Extract this into a private helper method (e.g., findKeyIndex()) that returns the index of a key or -1 if not found.
| public V get(K key) { | ||
| for (int i = 0; i < size; i++) { | ||
| if (Objects.equals(this.key[i], key)) { |
There was a problem hiding this comment.
Checklist item #5: Same repeating pattern issue as line 21. The key search loop exists in both put() and get() methods.
| private K [] key; | ||
| private V [] value; |
There was a problem hiding this comment.
The variable names 'key' and 'value' (lines 8-9) are not informative. Consider using plural forms like 'keys' and 'values' since they store multiple elements.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is now complete and meets all requirements. The code properly handles key-value storage using arrays with generics, correctly replaces values for duplicate keys, enforces the 10-element capacity limit via MAX_CAPACITY, and implements all required methods (put, get, size). The previous issues have been successfully resolved: direct null-safe comparison is used instead of Objects.equals(), and the duplicate key-search logic is cleanly extracted into the findIndex() helper method. All non-constant fields are properly initialized in the constructor. Great work on addressing all the feedback and producing clean, well-structured code!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.