Skip to content

made my own hashmap#1702

Open
StanislavKotsurskyi wants to merge 4 commits into
mate-academy:masterfrom
StanislavKotsurskyi:master
Open

made my own hashmap#1702
StanislavKotsurskyi wants to merge 4 commits into
mate-academy:masterfrom
StanislavKotsurskyi:master

Conversation

@StanislavKotsurskyi

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your implementation has a critical bug in the resize() method that breaks the getSize() functionality. When you set size = 0 before re-inserting elements, the put() method increments size for each element it adds. This causes the size counter to become approximately double the actual number of elements after resizing completes. For example, if you have 10 elements before resize, after rehashing you'll incorrectly report ~20 elements instead of 10. To fix this, simply remove the size = 0 line at line 65 — the size should remain unchanged during a resize operation since you're just reorganizing existing elements, not adding new ones. There's also a class structure violation: your private Node class appears after the private resize() and hash() methods. Per the checklist requirement, private classes should come after all methods (both public and private). Move the Node class to the end of the file after your private helper methods. The good news is that your core HashMap mechanics are solid — the hashing, collision handling via chaining, and resize trigger logic all work correctly. Fix these two issues and your implementation will be complete.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment on lines +64 to +65
private void resize() {
size = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical bug: Setting size = 0 before re-inserting elements corrupts the size counter. The put() method increments size for each element added, so after all re-insertions, size will only reflect the elements added during resize, not the total map size. Consider removing this line or tracking the original size separately.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your implementation has a critical functional bug that must be fixed before approval. In the resize() method (line 64), setting size = 0 causes a cascading problem: when elements are re-added during rehashing, each put() call increments size, resulting in approximately double the actual element count after every resize completes. Since elements are only being reorganized (not newly added), the size field should remain unchanged during resize—simply remove that line entirely. Additionally, the Node class at line 88 is missing the required private access modifier. Per the class structure requirements, internal utility classes should be private to maintain proper encapsulation. Change class Node<K, V> to private static class Node<K, V>. The rest of your implementation is solid—correct generics usage, proper load factor handling, and good variable naming. Once you fix these two issues, the code should work correctly.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

}

private void resize() {
size = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line resets size to 0, but subsequent put() calls during rehashing increment it for each element. After resize completes, size will be approximately double the actual count. Remove this line - size should remain unchanged during resize since elements are only reorganized, not newly added.

}
return Math.abs(key.hashCode() % capacity);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Node class is missing the private access modifier. Per checklist requirements, all classes/fields/methods should have access modifiers, and internal utility classes should be private to maintain encapsulation. Change to 'private static class Node<K, V>'.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your solution is approved with an educational note. The Node visibility fix from the previous review is well-implemented, and most of your HashMap follows the checklist requirements correctly, including named constants, proper access modifiers, and informative variable names. However, there's still one critical issue remaining in your resize() method at line 64—the size = 0; line needs to be removed entirely. Since resize() only reorganizes existing elements (they aren't new additions), resetting size to zero causes it to double each time because each put() call in the rehashing loop increments it back up. This is a subtle but important distinction in how HashMap implementations work, and fixing this will make your map fully functional. Keep up the good work on the implementation structure.


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

Footnotes

  1. Rate AI review example

}

private void resize() {
size = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the critical bug from the previous review. During resize, elements are being REORGANIZED (not newly added), so size should remain unchanged. Setting size = 0 causes the actual size to approximately double after each resize completes, because each put() call on line 75 increments size. Simply remove this line.

@Elena-Bruyako Elena-Bruyako left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants