hw for src/main/java/core/basesyntax/MyHashMap.java#1653
hw for src/main/java/core/basesyntax/MyHashMap.java#1653egorkharaim wants to merge 2 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good job on implementing the core functionality of the HashMap. The logic for handling collisions and resizing is correct. However, this submission requires changes because it violates two explicit checklist items.
- Remove Getters and Setters in
Node: TheNodeclass should not contain getters and setters. The outerMyHashMapclass can access the private fields of its inner class directly, so you should use direct field access (e.g.,currentNode.key) instead. [CHECKLIST ITEM #3] - Inner Class Placement: The private inner
Nodeclass must be moved to the end of theMyHashMapclass, after all public and private methods, to comply with the style guide. [CHECKLIST ITEM #5]
Please address these points to meet the task 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
| threshold = (int) (DEFAULT_CAPACITY * LOAD_FACTOR); | ||
| } | ||
|
|
||
| private static class Node<K, V> { |
There was a problem hiding this comment.
This violates checklist item #5: 'Private methods and classes should be after public ones in your class.' The inner Node class should be moved to the end of the MyHashMap class, after all public and private methods.
| public K getKey() { | ||
| return key; | ||
| } | ||
|
|
||
| public V getValue() { | ||
| return value; | ||
| } | ||
|
|
||
| public void setValue(V value) { | ||
| this.value = value; | ||
| } | ||
|
|
||
| public Node<K, V> getNext() { | ||
| return next; | ||
| } | ||
|
|
||
| public void setNext(Node<K, V> next) { | ||
| this.next = next; | ||
| } |
There was a problem hiding this comment.
This violates checklist item #3: 'Don't use getters and setters in class Node'. The outer MyHashMap class can access the private fields of its inner Node class directly. You should remove all these getter and setter methods and use direct field access (e.g., currentNode.key, currentNode.value = newValue) in your put, getValue, and resize methods.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this iteration! You've successfully addressed all the feedback from the previous review, and the result is a clean and compliant solution. Your MyHashMap implementation is solid, correctly handling collisions and resizing. It's great to see that the inner Node class is now implemented without getters and setters and is correctly placed at the end of the class. Your solution now meets all the requirements, so I am approving it. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.