MY HASHMAP #1589

bilsinas · 2025-08-09T18:02:05Z

No description provided.

mateacademy-ai-mentor

Review Summary

Great job on building a custom MyHashMap from scratch! You've correctly implemented the core structure and the put, getValue, and getSize methods. The code is well-organized and handles null keys correctly.

However, there are a couple of critical issues that need to be addressed before this can be approved.

Required Changes

Critical Bug in Hashing Logic

Your getIndex method has a bug that can cause a crash. The line Math.abs(key.hashCode()) will not work correctly for Integer.MIN_VALUE. Because of how two's complement integers work, the absolute value of Integer.MIN_VALUE is still a negative number, which will lead to an ArrayIndexOutOfBoundsException.
- Location: src/test/java/core/basesyntax/MyHashMap.java
- Impact: This is a critical bug that breaks the functionality of your map for certain hash codes.
- Suggestion: A safe way to ensure a non-negative index is to use a bitwise AND operation to remove the sign bit. This correctly handles all integer values, including Integer.MIN_VALUE.
```
// A robust way to calculate the index
return (key.hashCode() & 0x7fffffff) % table.length;
```
Inefficient Resizing Implementation

Your resize method works, but it's highly inefficient. Calling put() for every element during a resize is much slower than necessary. The put() method has to re-check for the key's existence and traverse the bucket's linked list, which is redundant work when you're just moving nodes to a new table.
- Location: src/test/java/core/basesyntax/MyHashMap.java
- Impact: This will cause significant performance issues as the map grows larger.
- Suggestion: You should implement resizing by iterating through the old table's nodes directly. For each node, calculate its new index in the new table and insert it at the head of that bucket's list. This is a much more performant approach.

Minor Improvements

Once you've fixed the issues above, consider these smaller improvements for code quality:

Avoid Magic Numbers: In your resize method, the number 2 is a magic number. It's better to declare it as a named constant, like private static final int RESIZE_MULTIPLIER = 2;.
Code Formatting: There are several extra empty lines in MyHashMap.java and MyMap.java that can be removed to improve readability.

You've built a solid foundation here. Addressing these points will make your MyHashMap much more robust and performant. 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 · 2025-08-09T18:05:18Z

src/test/java/core/basesyntax/MyHashMap.java

@@ -0,0 +1,96 @@
+package core.basesyntax;
+


Avoid redundant empty lines like this one. There are several of them in the file (e.g., lines 4, 8, 12, 17, etc.), which reduces readability.

mateacademy-ai-mentor · 2025-08-09T18:05:18Z

src/test/java/core/basesyntax/MyHashMap.java

+
+    private void resize() {
+        Node<K, V>[] oldTable = table;
+        int newCapacity = oldTable.length * 2;


It is a good practice to avoid magic numbers. Consider extracting 2 into a named constant like private static final int RESIZE_MULTIPLIER = 2;.

mateacademy-ai-mentor · 2025-08-09T18:05:19Z

src/test/java/core/basesyntax/MyHashMap.java

+
+        for (Node<K, V> headNode : oldTable) {
+            while (headNode != null) {
+                put(headNode.key, headNode.value);


Calling put() to rehash elements during resize is inefficient. Each call recalculates the hash and traverses the new bucket's list. A more optimal approach is to iterate through the old nodes, calculate their new index, and insert them directly into the new table array.

mateacademy-ai-mentor · 2025-08-09T18:05:19Z

src/test/java/core/basesyntax/MyHashMap.java

+        if (key == null) {
+            return 0;
+        }
+        return Math.abs(key.hashCode() % length);


key.hashCode() can return Integer.MIN_VALUE. Math.abs(Integer.MIN_VALUE) will also be a negative number, which will lead to an ArrayIndexOutOfBoundsException. A safer way to ensure a non-negative hash code is to use a bitwise AND operation to remove the sign bit, for example: (key.hashCode() & 0x7fffffff) % length.

mateacademy-ai-mentor · 2025-08-09T18:05:19Z

src/test/java/core/basesyntax/MyHashMap.java

+        K key;
+        V value;
+        Node<K, V> next;


For better encapsulation, it is good practice to declare class fields as private. Additionally, the key of a map entry is immutable by contract, so you should declare it as final.

mateacademy-ai-mentor · 2025-08-09T18:05:19Z

src/test/java/core/basesyntax/MyMap.java

@@ -0,0 +1,9 @@
+package core.basesyntax;
+


Avoid a redundant empty line after the package declaration.

mateacademy-ai-mentor · 2025-08-09T18:05:19Z

src/test/java/core/basesyntax/MyMap.java

+
+    V getValue(K key);
+


Avoid redundant empty lines between method declarations. A single empty line is sufficient to separate them.

mateacademy-ai-mentor

Review Summary

Great job on this implementation! You've correctly built the core functionality for put, getValue, and getSize, and the overall structure is very solid. It's clear you have a strong grasp of how HashMaps work.

However, there is one critical logical issue in the resize method that needs to be corrected before this can be approved.

Required Change

Issue: Incorrect size management during resize.
Location: src/test/java/core/basesyntax/MyHashMap.java

Description: In your resize() method, you reset size to 0 and then increment it for every element you move to the new table. This is a logical error because the number of elements in the map does not change during a resize operation. This makes the map's state temporarily inconsistent and is a conceptual flaw.

To fix this, the size of the map should remain constant throughout the resize. You can simply remove the lines that modify the size field within the resize method.

// src/test/java/core/basesyntax/MyHashMap.java
private void resize() {
    Node<K, V>[] oldTable = table;
    int newCapacity = oldTable.length * RESIZE_MULTIPLIER;
    table = new Node[newCapacity];
    threshold = (int) (newCapacity * DEFAULT_LOAD_FACTOR);
    // size = 0; // <<< REMOVE THIS LINE

    for (Node<K, V> headNode : oldTable) {
        while (headNode != null) {
            Node<K, V> nextNode = headNode.next;
            int index = getIndex(headNode.key, newCapacity);
            headNode.next = table[index];
            table[index] = headNode;
            // size++; // <<< REMOVE THIS LINE
            headNode = nextNode;
        }
    }
}

Minor Suggestions

Once you've fixed the issue above, consider also removing the redundant empty lines in both MyHashMap.java and MyMap.java to improve code style.

You're very close to a perfect solution! Address the resize logic, and you'll be good to go. Keep up the excellent 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 · 2025-08-09T18:23:43Z

src/test/java/core/basesyntax/MyHashMap.java

@@ -0,0 +1,101 @@
+package core.basesyntax;
+


There are several redundant empty lines throughout the file (e.g., here, and on lines 4, 9, 13, etc.). Please remove them to improve code readability and adhere to the style guide.

mateacademy-ai-mentor · 2025-08-09T18:23:43Z

src/test/java/core/basesyntax/MyHashMap.java

+        int newCapacity = oldTable.length * RESIZE_MULTIPLIER;
+        table = new Node[newCapacity];
+        threshold = (int) (newCapacity * DEFAULT_LOAD_FACTOR);
+        size = 0;


In the resize method, resetting size to 0 and then incrementing it for each element is unnecessary. The number of elements in the map does not change during a resize operation. You can simplify the method by removing this line and the size++ line (line 77) within the loop.

mateacademy-ai-mentor · 2025-08-09T18:23:43Z

src/test/java/core/basesyntax/MyMap.java

@@ -0,0 +1,9 @@
+package core.basesyntax;
+


There are several redundant empty lines in the interface definition (e.g., here, and on lines 5 and 7). Please remove them to make the code more compact and clean, following the coding style guidelines.

mateacademy-ai-mentor

Review Summary

Great work on this MyHashMap implementation! This is an excellent submission that correctly handles all the core requirements, including generics, collision handling, and resizing. The code is clean, well-organized, and demonstrates a solid understanding of how hash maps work.

I'm approving this to support your progress. I have a few suggestions below that can help you refine your implementation even further and introduce you to some common optimizations and design patterns.

Suggestions for Improvement

Here are a few areas you could improve:

1. Simplifying Node Insertion in `put()`

The logic for adding a new element to a bucket can be simplified. Currently, you traverse the linked list to add a new node at the end. A more common and slightly more efficient approach is to add new nodes to the beginning of the list.

Why it's better: This avoids the need to iterate to the end of the list and simplifies the code by removing the prev variable.

How to do it: You could refactor this block to first iterate through the list to check if the key already exists. If not, you can add the new node at the head of the bucket like this:

// Inside the put method, after finding the correct bucket (index)
// and confirming the key is new
Node<K, V> newNode = new Node<>(key, value, table[index]);
table[index] = newNode;
size++;

2. Unnecessary `size` Manipulation in `resize()`

In your resize() method, you reset size to 0 and then increment it for every element that is re-hashed.

Why it's an issue: A resize operation simply redistributes existing elements; it doesn't change their count. While this doesn't lead to an incorrect final size, it's logically redundant. The size of the map should be constant during a resize.

How to fix it: The size field should only be modified by operations that add or remove elements (like put). You can safely remove size = 0; and size++; from the resize() method.

// In src/test/java/core/basesyntax/MyHashMap.java
private void resize() {
    Node<K, V>[] oldTable = table;
    // ...
    // size = 0; // This line should be removed
    for (Node<K, V> headNode : oldTable) {
        while (headNode != null) {
            // ... rehashing logic ...
            // size++; // This line should be removed
            headNode = nextNode;
        }
    }
}

3. Strengthening Encapsulation for the `Node` Class

The Node constructor has default (package-private) access.

Why it's better to change: Since Node is a private static inner class and its constructor is only ever used from within the enclosing MyHashMap class, you can declare the constructor as private. This is a good practice that strengthens encapsulation.
How to fix it: Simply add the private modifier to the Node constructor.

4. Performance Tip: Index Calculation

This is a minor optimization but a great concept to learn. You are using the modulo operator (%) to calculate the bucket index. Because your table capacity is always a power of two (16, 32, 64...), a bitwise AND operation (&) can be used instead, which is typically faster.

How to do it: You can replace the modulo operation with a bitwise AND. This is a common optimization in HashMap implementations.

// Suggestion for getIndex()
private int getIndex(K key, int length) {
    if (key == null) {
        return 0;
    }
    // Using bitwise AND is faster than modulo when length is a power of two.
    // The `& 0x7fffffff` ensures the hash code is positive before the operation.
    return (key.hashCode() & 0x7fffffff) & (length - 1);
}

Overall, this is a fantastic piece of work. These suggestions are aimed at refining an already strong implementation. Keep up the great effort

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-08-09T18:38:34Z

src/test/java/core/basesyntax/MyHashMap.java

+        Node<K, V> current = table[index];
+        if (current == null) {
+            table[index] = new Node<>(key, value, null);
+            size++;
+            return;
+        }
+        Node<K, V> prev = null;
+        while (current != null) {
+            if (Objects.equals(current.key, key)) {
+                current.value = value;
+                return;
+            }
+            prev = current;
+            current = current.next;
+        }
+        prev.next = new Node<>(key, value, null);
+        size++;


The logic for adding a new element to a bucket can be simplified. Currently, you are traversing the linked list to add a new node at the end. A more common and slightly more efficient approach is to add new nodes to the beginning of the list. This avoids the need to iterate to the end and simplifies the code by removing the prev variable and the special check for an empty bucket. You could refactor this block to first iterate through the list to check if the key already exists, and if not, add the new node at the head of the bucket.

mateacademy-ai-mentor · 2025-08-09T18:38:34Z

src/test/java/core/basesyntax/MyHashMap.java

+        private final K key;
+        private V value;
+        private Node<K, V> next;
+        Node(K key, V value, Node<K, V> next) {


The Node constructor has default (package-private) access. Since Node is a private static inner class and its constructor is only invoked from within the enclosing MyHashMap class, you can declare the constructor as private to strengthen encapsulation.

MY HASHMAP

26731c8

mateacademy-ai-mentor suggested changes Aug 9, 2025

View reviewed changes

MY HASHMAP

2156315

bilsinas requested a review from mateacademy-ai-mentor August 9, 2025 18:21

mateacademy-ai-mentor suggested changes Aug 9, 2025

View reviewed changes

MY HASHMAP

3157e7d

bilsinas requested a review from mateacademy-ai-mentor August 9, 2025 18:35

mateacademy-ai-mentor approved these changes Aug 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MY HASHMAP #1589

MY HASHMAP #1589

Uh oh!

bilsinas commented Aug 9, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

mateacademy-ai-mentor Aug 9, 2025

Uh oh!

Uh oh!

MY HASHMAP #1589

Are you sure you want to change the base?

MY HASHMAP #1589

Uh oh!

Conversation

bilsinas commented Aug 9, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Required Changes

Minor Improvements

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!

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

Required Change

Minor Suggestions

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

Suggestions for Improvement

1. Simplifying Node Insertion in put()

2. Unnecessary size Manipulation in resize()

3. Strengthening Encapsulation for the Node Class

4. Performance Tip: Index Calculation

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

1. Simplifying Node Insertion in `put()`

2. Unnecessary `size` Manipulation in `resize()`

3. Strengthening Encapsulation for the `Node` Class