i have done#1913
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 several critical checklist violations that must be fixed before approval. First, you use java.util.Objects.equals() which explicitly violates the 'Don't use class Objects' requirement [CHECKLIST ITEM #X] - replace it with a manual null-safe comparison method. Second, your non-constant fields (keys, values, size) are initialized as field declarations, but the checklist requires all non-constant fields to be initialized in the constructor [CHECKLIST ITEM #X]. Third, the constant MAX lacks the static modifier and has a non-informative name - it should be private static final int MAX_STORAGE_CAPACITY = 10;. Additionally, remove the redundant empty lines at the start of your method implementations. While your core logic for put(), get(), and size() operations is functionally correct, these explicit checklist violations are blockers that must be addressed.
✨ 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
| @@ -1,19 +1,53 @@ | |||
| package core.basesyntax.impl; | |||
|
|
|||
| import java.util.Objects; | |||
There was a problem hiding this comment.
Uses java.util.Objects class which violates 'Don't use class Objects' requirement. Replace Objects.equals() with a manual null-safe comparison method.
|
|
||
| for (int i = 0; i < size; i++) { | ||
|
|
||
| if (Objects.equals(keys[i], key)) { |
There was a problem hiding this comment.
Replaces existing key's value - uses Objects.equals which is not allowed per checklist requirements.
| public V get(K key) { | ||
|
|
||
| for (int i = 0; i < size; i++) { | ||
|
|
There was a problem hiding this comment.
Replaces existing key's value - uses Objects.equals which is not allowed per checklist requirements.
|
|
||
| public class StorageImpl<K, V> implements Storage<K, V> { | ||
|
|
||
| private final int MAX = 10; |
There was a problem hiding this comment.
MAX is declared as instance field instead of static constant. Should be 'private static final int MAX_STORAGE_CAPACITY = 10;'
|
|
||
| private final int MAX = 10; | ||
|
|
||
| Object[] keys = new Object[MAX]; |
There was a problem hiding this comment.
Non-constant field 'keys' is initialized as field declaration. Per checklist: all non-constant fields should be initialized in the constructor.
| private final int MAX = 10; | ||
|
|
||
| Object[] keys = new Object[MAX]; | ||
| Object[] values = new Object[MAX]; |
There was a problem hiding this comment.
Non-constant field 'values' is initialized as field declaration. Per checklist: all non-constant fields should be initialized in the constructor.
|
|
||
| Object[] keys = new Object[MAX]; | ||
| Object[] values = new Object[MAX]; | ||
| int size = 0; |
There was a problem hiding this comment.
Non-constant field 'size' is initialized as field declaration. Per checklist: all non-constant fields should be initialized in the constructor.
| public void put(K key, V value) { | ||
|
|
There was a problem hiding this comment.
Empty line at start of method implementation - remove redundant empty line.
| @Override | ||
| public V get(K key) { |
There was a problem hiding this comment.
Empty line at start of method implementation - remove redundant empty line.
| } | ||
| return null; | ||
|
|
||
| } |
There was a problem hiding this comment.
Empty line at start of method implementation - remove redundant empty line.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation correctly fulfills all functional requirements: the storage uses two arrays with a capacity of 10, put(), get(), and size() methods work as expected, and you've properly avoided using java.util.Objects, List, Set, or Map collections. There's one remaining issue regarding [CHECKLIST ITEM #4]: the null-safe equality check expression keys[i] == null ? key == null : keys[i].equals(key) appears identically in both put() (around line 25) and get() methods. While this duplication doesn't break functionality, extracting this into a private helper method like private boolean keysEqual(Object storedKey, Object searchKey) would follow the DRY principle and align with the checklist requirement. Consider making this small refactor to achieve full compliance with the checklist, but know that your core implementation is solid and working correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| public void put(K key, V value) { | ||
| for (int i = 0; i < size; i++) { | ||
|
|
||
| if (keys[i] == null ? key == null : keys[i].equals(key)) { |
There was a problem hiding this comment.
CHECKLIST VIOLATION #4: This null-safe equality check is duplicated in get() method at line 39. Extract this logic into a private helper method (e.g., areKeysEqual(Object storedKey, K searchKey)) to follow the DRY principle.
2 participants
No description provided.