Skip to content

done#1698

Open
acidcor wants to merge 5 commits into
mate-academy:masterfrom
acidcor:master
Open

done#1698
acidcor wants to merge 5 commits into
mate-academy:masterfrom
acidcor:master

Conversation

@acidcor

@acidcor acidcor commented Apr 15, 2026

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

Changes are needed before this solution can be accepted, mainly because several explicit checklist requirements are not yet satisfied even though the core put, getValue, and getSize logic is implemented correctly. The first issue is member ordering in MyHashMap.java: the private helper methods like addNewNode(...), resize(...), and getIndexByKey(...) are declared before the public API methods (put, getValue, getSize), which violates [CHECKLIST ITEM #4]; please move all public methods above private ones. The second issue is missing access modifiers on the MyHashMap() constructor, which currently has default/package-private visibility; add an explicit modifier (typically public for this task) to comply with [CHECKLIST ITEM #7]. The third issue is that the inner Node class is declared without an access modifier; declare it with one (for example private static class Node<K, V>) to respect encapsulation and [CHECKLIST ITEM #7]. Finally, the constant mask 0x7fffffff used in your hash/index calculation is a magic number; extract it into a named constant such as private static final int HASH_MASK = 0x7fffffff; and use that instead, to satisfy [CHECKLIST ITEM #1]. Once you address these style/checklist violations, your implementation will align both with the functional requirements and the formal checklist, and it should be ready to pass review.


✨ 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

buckets = new Node[initialCapacity];
}

private void addNewNode(Node<K, V> newNode) {

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 violates checklist item #4: "Private methods and classes should be after public ones in your class." You declare private helper methods (addNewNode, resize, getIndexByKey) before the public API methods (put, getValue, getSize). Move private methods after the public methods so private members come after public ones.

private Node<K, V>[] buckets;
private int size = 0;

MyHashMap() {

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 violates checklist item #7: "Remember about access modifiers. All fields and methods should have access modifiers. Remember about the encapsulation principle." The constructor MyHashMap() has no explicit access modifier. Add an appropriate modifier (e.g., public or private) to comply with the checklist.

return size;
}

class Node<K, V> {

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 also violates checklist item #7: the inner class Node has no explicit access modifier. Declare it with an access modifier (for example private static class Node<K, V>) to respect encapsulation and the checklist requirement.

}

int hash = key.hashCode();
return (hash & 0x7fffffff) % currentMaxSize;

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 violates checklist item #1: "Any magic numbers should be constants" — the mask 0x7fffffff is a magic number. Move such literals to a named constant (for example private static final int HASH_MASK = 0x7fffffff;) to improve readability and follow the checklist.

@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.

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

@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.

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

@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

I am requesting changes before this solution can be accepted.

The core HashMap functionality (put, getValue, getSize, collision handling, and resizing) looks correct, but there is still one explicit checklist violation that must be fixed: the magic number 0x7fffffff is used directly in getIndexByKey(...) (in the line with int hashMask = 0x7fffffff;).
According to the checklist, any hardcoded numerical values should be moved to named constants to keep the code self-documenting and easier to maintain.
To fix this, introduce a class-level constant like private static final int HASH_MASK = 0x7fffffff; and replace the local hashMask value in getIndexByKey(...) with that constant.
This is the only blocking issue I see; once you extract that magic number into a well-named constant, your implementation will fully meet the stated checklist requirements.
Overall, your solution is structurally sound and close to final—this is a small but important polish step that will help you build good habits for future tasks.


✨ 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

}

int hash = key.hashCode();
int hashMask = 0x7fffffff;

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 violates checklist item: "Any magic numbers should be constants" — the mask 0x7fffffff is used as a local magic number (line uses int hashMask = 0x7fffffff;). Extract it to a named constant (for example private static final int HASH_MASK = 0x7fffffff;) and use that constant in getIndexByKey(...).

@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

I am approving your solution as an educational override so you can continue progressing while you practice polishing code style and encapsulation. Functionally, your MyHashMap implementation is solid: put, getValue, getSize, load factor handling, and resizing with rehashing are all implemented correctly and clearly. There are a couple of style issues to fix next time: in MyHashMap.java, remove the redundant blank line right after the package declaration so the class declaration starts immediately on the next line, and remove the blank line immediately before the constructor inside the class body to satisfy [CHECKLIST ITEM #1]. Also, in the inner Node<K, V> class, the constructor Node(K key, V value) currently has no explicit access modifier; per [CHECKLIST ITEM #7], mark it explicitly (for example, private Node(K key, V value)) to keep the node implementation properly encapsulated. As a minor improvement, consider renaming your constants to the conventional upper-case style (e.g., HASH_MASK instead of hashMask) to better match common Java conventions. Overall, your logic and structure are strong; focusing on these small formatting and access-modifier details will bring your solution fully in line with professional Java style.


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

Footnotes

  1. Rate AI review example

@@ -1,19 +1,148 @@
package core.basesyntax;

public class MyHashMap<K, V> implements MyMap<K, V> {

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 violates checklist item #1: "Don't begin class or method implementation with empty line. Remove all redundant empty lines, be careful :)" — there is an empty line immediately before the class declaration. Remove the redundant blank line so the class does not begin with an empty line.

private Node<K, V>[] buckets;
private int size = 0;

public MyHashMap() {

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 violates checklist item #1: "Don't begin class or method implementation with empty line. Remove all redundant empty lines, be careful :)" — there is an empty line before the constructor. Remove the redundant blank line so the constructor does not begin with an empty line.

private V value;
private Node<K, V> next;

Node(K key, V value) {

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 violates checklist item #7: "Remember about access modifiers. All fields and methods should have access modifiers. Remember about the encapsulation principle." — the Node constructor Node(K key, V value) has no explicit access modifier. Add an access modifier (for example private Node(...)) to follow the encapsulation requirement.

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.

2 participants