Jv storage#1916
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The solution correctly implements the core functional requirements for the Storage interface. The StorageImpl class properly handles put, get, and size operations using arrays, respects the 10-element capacity limit, and follows the required type parameters. However, there are several checklist violations that should be addressed: [CHECKLIST #2] Field initialization: Move size = 0 from the field declaration to the constructor. The size field should be declared without initialization at the field level. [CHECKLIST #5] DRY principle: The null-checking pattern for key comparison is duplicated in both put() and get() methods. Extract this logic into a private helper method (e.g., private int findKeyIndex(K key)) that returns the index of an existing key or -1 if not found. [CHECKLIST #3] Constant naming: Rename MAX_Keys_And_Values to MAX_KEYS_AND_VALUES following UPPER_SNAKE_CASE convention. Consider a more descriptive name like MAX_STORAGE_CAPACITY. Logical inconsistency: The get() method iterates using keys.length while put() uses size. For consistency and efficiency, get() should iterate using size to avoid checking uninitialized array positions. Formatting: Remove empty lines at the class start and before method implementations. The code structure is sound and the implementation is functional—these are refinements rather than fundamental issues.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import core.basesyntax.Storage; | ||
|
|
||
| public class Main extends StorageImpl { | ||
| public static void main(String[] args) { |
There was a problem hiding this comment.
Checklist item #3 violation: Constant name should follow UPPER_SNAKE_CASE naming convention (e.g., MAX_ELEMENTS_CAPACITY) and could be more informative about its purpose.
| @@ -3,17 +3,47 @@ | |||
| import core.basesyntax.Storage; | |||
|
|
|||
There was a problem hiding this comment.
Empty line before class declaration. Remove according to checklist requirements.
| public static final int MAX_Keys_And_Values = 10; | ||
| protected final K[] keys; | ||
| protected final V[] values; | ||
| private int size = 0; |
There was a problem hiding this comment.
All non-constant fields should be initialized in the constructor. Move size = 0 initialization from field declaration (line 9) to the constructor.
| protected final K[] keys; | ||
| protected final V[] values; | ||
| private int size = 0; | ||
|
|
There was a problem hiding this comment.
Empty line before method implementation. Remove according to checklist.
| this.keys = (K[]) new Object[MAX_Keys_And_Values]; | ||
| this.values = (V[]) new Object[MAX_Keys_And_Values]; | ||
| } | ||
|
|
There was a problem hiding this comment.
Empty line before method implementation. Remove according to checklist.
| values[size] = value; | ||
| size++; | ||
| } | ||
|
|
There was a problem hiding this comment.
Empty line before method implementation. Remove according to checklist.
|
|
||
| @Override | ||
| public V get(K key) { | ||
| for (int i = 0; i < keys.length; i++) { |
There was a problem hiding this comment.
Logical inconsistency: put() iterates using size while get() uses keys.length. When checking for keys, get() should use size to avoid accessing uninitialized array positions.
| } | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Empty line before method implementation. Remove according to checklist.
No description provided.