-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Maksym Gorodnychyi #1910
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
base: master
Are you sure you want to change the base?
Maksym Gorodnychyi #1910
Changes from 4 commits
68d5332
cdcac41
a127230
ee7942e
6b53c3c
a360d0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package core.basesyntax; | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #1: "Don't begin class or method implementation with empty line. Remove all redundant empty lines, be careful :)" — remove the empty line immediately after the package declaration so the import/class declaration starts right after the package line. |
||
| public class StorageImpl<K, V> implements Storage<K, V> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why there is 2 StorageImpl classes? |
||
| private static final int MAX_SIZE = 10; | ||
| private K[] keys; | ||
| private V[] values; | ||
| private int size; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CHECKLIST ITEM #2 VIOLATION: The |
||
|
|
||
| @SuppressWarnings("unchecked") | ||
| public StorageImpl() { | ||
| keys = (K[]) new Object[MAX_SIZE]; | ||
| values = (V[]) new Object[MAX_SIZE]; | ||
|
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #6: "Don't use class Objects". The constructor creates generic arrays via
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CHECKLIST ITEM #6 VIOLATION: Generic arrays should not be created via direct |
||
| } | ||
|
Comment on lines
+7
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #2: "All non-constant fields should be initialized in the constructor". The field |
||
|
|
||
| @Override | ||
| public void put(K key, V value) { | ||
| for (int i = 0; i < size; i++) { | ||
| boolean keysAreEqual = keys[i] == null ? key == null : keys[i].equals(key); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #5 (DRY principle). The key comparison logic |
||
| if (keysAreEqual) { | ||
| values[i] = value; | ||
| return; | ||
| } | ||
| } | ||
| keys[size] = key; | ||
| values[size] = value; | ||
| size++; | ||
| } | ||
|
|
||
| @Override | ||
| public V get(K key) { | ||
| for (int i = 0; i < size; i++) { | ||
| boolean keysAreEqual = keys[i] == null ? key == null : keys[i].equals(key); | ||
|
Comment on lines
+18
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CHECKLIST ITEM #5 VIOLATION: The key comparison logic |
||
| if (keysAreEqual) { | ||
|
Comment on lines
+18
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #5: "Don't create repeating code If the logic of your code repeats - move it to the separate private method." The key-comparison expression |
||
| return values[i]; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public int size() { | ||
| return size; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,17 +3,44 @@ | |||||||
| import core.basesyntax.Storage; | ||||||||
|
|
||||||||
| public class StorageImpl<K, V> implements Storage<K, V> { | ||||||||
| private static final int MAX_SIZE = 10; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #1: "Don't begin class or method implementation with empty line. Remove all redundant empty lines, be careful :)" Remove the empty line after the package/import block so the class declaration and imports start without leading blank lines. |
||||||||
| private K[] keys; | ||||||||
| private V[] values; | ||||||||
| private int size; | ||||||||
|
|
||||||||
| @SuppressWarnings("unchecked") | ||||||||
| public StorageImpl() { | ||||||||
|
Comment on lines
+8
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #2: "All non-constant fields should be initialized in the constructor". The field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checklist item #2 violation: The 'size' field should be explicitly initialized in the constructor. Add 'this.size = 0;' inside the constructor. |
||||||||
| keys = (K[]) new Object[MAX_SIZE]; | ||||||||
| values = (V[]) new Object[MAX_SIZE]; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #6: "Don't use class Objects". The constructor creates arrays via |
||||||||
| } | ||||||||
|
|
||||||||
| @Override | ||||||||
| public void put(K key, V value) { | ||||||||
| for (int i = 0; i < size; i++) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checklist item #5 violation: Key comparison logic duplicated in put() method (lines 27-30). Extract to a private helper method like 'private boolean areKeysEqual(K a, K b)' to follow DRY principle. |
||||||||
| boolean keysAreEqual = keys[i] == null ? key == null : keys[i].equals(key); | ||||||||
| if (keysAreEqual) { | ||||||||
| values[i] = value; | ||||||||
| return; | ||||||||
| } | ||||||||
| } | ||||||||
| keys[size] = key; | ||||||||
| values[size] = value; | ||||||||
| size++; | ||||||||
| } | ||||||||
|
|
||||||||
| @Override | ||||||||
| public V get(K key) { | ||||||||
| for (int i = 0; i < size; i++) { | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checklist item #5 violation: Same key comparison logic duplicated in get() method. Extract to the same private helper method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. create
Suggested change
|
||||||||
| boolean keysAreEqual = keys[i] == null ? key == null : keys[i].equals(key); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This violates checklist item #5: "Don't create repeating code If the logic of your code repeats - move it to the separate private method." The key-comparison expression |
||||||||
| if (keysAreEqual) { | ||||||||
| return values[i]; | ||||||||
| } | ||||||||
| } | ||||||||
| return null; | ||||||||
| } | ||||||||
|
|
||||||||
| @Override | ||||||||
| public int size() { | ||||||||
| return -1; | ||||||||
| return size; | ||||||||
| } | ||||||||
| } | ||||||||
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.
Remove this empty line. Per checklist item #1, do not begin class implementation with empty line.