first solution by mate-student26 · Pull Request #1686 · mate-academy/jv-hash-map

mate-student26 · 2026-04-08T20:38:51Z

No description provided.

mateacademy-ai-mentor

Review Summary

Your MyHashMap implementation is functionally correct, which is a great start! However, changes are needed to align your code with the project's specific requirements. Please address the following points:

The Node inner class currently has getters and setters, which violates a specific project requirement. You should remove these methods and access the node's fields directly from the outer class. [CHECKLIST ITEM #3]
The value 0x7fffffff is used as a magic number. Please extract it into a private static final constant with a descriptive name. [CHECKLIST ITEM #2]
The private Node class and other private helper methods should be moved to the bottom of the MyHashMap class, after all public methods. The Node class should also be declared private. [CHECKLIST ITEM #5]

Once these adjustments are made, your solution will be in great shape!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-04-08T20:41:00Z

 public class MyHashMap<K, V> implements MyMap<K, V> {
+    private static final int DEFAULT_INITIAL_CAPACITY = 16;
+    private static final float DEFAULT_LOAD_FACTOR = 0.75f;
+    static class Node<K, V> implements Map.Entry<K, V> {


This violates checklist item #5: 'Private methods and classes should be after public ones in your class.' and #7: 'Remember about access modifiers.' The Node class should be declared private since it is only used within MyHashMap. It should also be moved to the bottom of the MyHashMap class, after all public methods.

mateacademy-ai-mentor · 2026-04-08T20:41:00Z

+        public K getKey() {
+            return key;
+        }
+
+        @Override
+        public V getValue() {
+            return value;
+        }
+
+        @Override
+        public V setValue(V value) {
+            V oldValue = this.value;
+            this.value = value;
+            return oldValue;
+        }


This violates checklist item #3: 'Don't use getters and setters in class Node'. While implementing Map.Entry requires these methods, the task requirements do not require you to implement this interface. Please remove implements Map.Entry<K, V> from the Node class definition and delete these getter/setter methods to comply with the checklist.

mateacademy-ai-mentor · 2026-04-08T20:41:00Z

+        threshold = (int) (DEFAULT_INITIAL_CAPACITY * DEFAULT_LOAD_FACTOR);
+    }
+
+    private int hash(K key) {


This violates checklist item #5: 'Private methods and classes should be after public ones in your class.' Private helper methods should be placed after all public methods in the class.

mateacademy-ai-mentor · 2026-04-08T20:41:00Z

+            resize();
+        }
+        int hash = hash(key);
+        int index = (hash & 0x7fffffff) % table.length;


This violates checklist item #2: 'Any magic numbers should be constants'. The value 0x7fffffff is a magic number used to ensure the hash code is positive. It appears in multiple places and should be extracted into a private static final constant with a descriptive name.

mateacademy-ai-mentor

Review Summary

Great job on the core logic of your MyHashMap! However, this submission requires changes because several key requirements from the project checklist have not been met. These were also mentioned in the previous review.

Please address the following:

Magic Number: The value 0x7fffffff is used directly in your code. Please extract it into a private static final constant with a descriptive name. [CHECKLIST ITEM #2]
Node Class Implementation: The Node class should not have getters or setters. To fix this, remove implements Map.Entry<K, V> and the getKey(), getValue(), and setValue() methods. [CHECKLIST ITEM #3]
Class Structure: All private members, including the Node class and any private helper methods, must be placed after all public methods in the class file. [CHECKLIST ITEM #5]

Once these checklist items are addressed, your solution will be in great shape.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-04-08T20:59:59Z

+    private static final int DEFAULT_INITIAL_CAPACITY = 16;
+    private static final float DEFAULT_LOAD_FACTOR = 0.75f;
+
+    static class Node<K, V> implements Map.Entry<K, V> {


This Node class should be declared private and moved to the end of the MyHashMap class, after all public methods. Additionally, per checklist item #3, it should not contain getters and setters. Implementing Map.Entry forces you to add these methods, but the project requirements take precedence. Please remove the implements Map.Entry<K, V> part and the corresponding getKey, getValue, and setValue methods.

mateacademy-ai-mentor · 2026-04-08T20:59:59Z

+        threshold = (int) (DEFAULT_INITIAL_CAPACITY * DEFAULT_LOAD_FACTOR);
+    }
+
+    private int hash(K key) {


This violates checklist item #5: 'Private methods and classes should be after public ones in your class.' Please move this private helper method to a position after all public methods like put, getValue, and getSize.

mateacademy-ai-mentor · 2026-04-08T20:59:59Z

+            resize();
+        }
+        int hash = hash(key);
+        int index = (hash & 0x7fffffff) % table.length;


This violates checklist item #2: 'Any magic numbers should be constants'. The value 0x7fffffff is a magic number and should be extracted into a private static final constant with a descriptive name. This value is used in multiple places (getValue and resize), and using a constant will improve readability and maintainability.

mateacademy-ai-mentor

Review Summary

I am approving this submission as an educational override to help you progress. Great work on refactoring the Node class—your core logic is looking very strong!

For your next submission, please focus on a couple of key style guide rules:

Class Structure: All private members, including the inner Node class and helper methods like hash(), should be placed at the end of the file, after all public methods. [CHECKLIST ITEM #5]
Magic Numbers: Values like Integer.MAX_VALUE should be extracted into private static final constants with descriptive names to make their purpose clear. [CHECKLIST ITEM #2]

Also, be sure to remove any unused imports, as one is causing the build to fail. Addressing these points will make your excellent logic even more professional. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-04-08T21:03:05Z

+    private static final float DEFAULT_LOAD_FACTOR = 0.75f;
+
+    private class Node<K, V> {
+        private final int hash;


This violates checklist item #5: 'Private methods and classes should be after public ones in your class.' Please move the private Node inner class and the private hash() method to the end of the MyHashMap class, after all public methods.

mateacademy-ai-mentor · 2026-04-08T21:03:05Z

+        }
+        int hash = hash(key);
+        int index = (hash & Integer.MAX_VALUE) % table.length;
+        Node<K, V> current = table[index];


This violates checklist item #2: 'Any magic numbers should be constants'. While using Integer.MAX_VALUE is better than a raw hex value, the requirement is to create your own private static final constant with a descriptive name (e.g., POSITIVE_INTEGER_MASK) to make the intent of the bitwise operation clear.

slade13

Your code does not build: https://github.com/mate-academy/jv-hash-map/actions/runs/24158894003/job/70504942928?pr=1686

Error: /home/runner/work/jv-hash-map/jv-hash-map/src/main/java/core/basesyntax/MyHashMap.java:89:5: Instance variable definition in wrong order. [DeclarationOrder]
Error: /home/runner/work/jv-hash-map/jv-hash-map/src/main/java/core/basesyntax/MyHashMap.java:90:5: Instance variable definition in wrong order. [DeclarationOrder]
Error: /home/runner/work/jv-hash-map/jv-hash-map/src/main/java/core/basesyntax/MyHashMap.java:91:5: Instance variable definition in wrong order. [DeclarationOrder]

Please fix it and make sure that it builds successfully with command mvn clean verify before sending it for review.

mateuszwojtkowiak

Your project did not build with Maven correctly. Please run mvn clean package and fix all errors.

slade13

Good work, please review my comments and add necessary changes. Besides these small problems your code looks good and follows the task requirements.

slade13 · 2026-06-03T07:27:43Z

+        return key == null ? 0 : key.hashCode();
+    }
+
+    private class Node<K, V> {


Node should be static:

Suggested change

private class Node<K, V> {

private static class Node<K, V> {

Since Node does not need access to the outer MyHashMap instance, making it static avoids storing an unnecessary reference to the outer object.

slade13 · 2026-06-03T07:28:46Z

+        private V value;
+        private Node<K, V> next;
+
+        public Node(int hash, K key, V value, Node<K, V> next) {


Since Node is used only inside MyHashMap, the constructor should also be restricted:

Suggested change

public Node(int hash, K key, V value, Node<K, V> next) {

private Node(int hash, K key, V value, Node<K, V> next) {

slade13 · 2026-06-03T07:30:00Z

+            resize();
+        }
+        int hash = hash(key);
+        int index = (hash & HASH_MASK) % table.length;


Index calculation is duplicated, this appears in more than one place:

int index = (hash & HASH_MASK) % table.length;

It would be cleaner to extract it into a private helper method, for example:

private int getIndex(int hash, int length)

first solution

e9680d1

mateacademy-ai-mentor suggested changes Apr 8, 2026

View reviewed changes

checkstyle fixes

607cd32

mate-student26 requested a review from mateacademy-ai-mentor April 8, 2026 20:58

mateacademy-ai-mentor suggested changes Apr 8, 2026

View reviewed changes

after fixes

f03378a

mate-student26 requested a review from mateacademy-ai-mentor April 8, 2026 21:01

after fixes2

f43dfe4

mateacademy-ai-mentor suggested changes Apr 8, 2026

View reviewed changes

after fixes3

5e508f8

mate-student26 requested a review from mateacademy-ai-mentor April 8, 2026 21:10

slade13 suggested changes Apr 9, 2026

View reviewed changes

mate-student26 requested a review from slade13 June 2, 2026 16:53

mateuszwojtkowiak suggested changes Jun 2, 2026

View reviewed changes

added missing test components

e0b7101

mate-student26 requested a review from mateuszwojtkowiak June 2, 2026 17:04

slade13 approved these changes Jun 3, 2026

View reviewed changes

	public Node(int hash, K key, V value, Node<K, V> next) {
	private Node(int hash, K key, V value, Node<K, V> next) {

Conversation

mate-student26 commented Apr 8, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slade13 left a comment

Choose a reason for hiding this comment

Uh oh!

mateuszwojtkowiak left a comment

Choose a reason for hiding this comment

Uh oh!

slade13 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants