Skip to content

done hashmap task#1663

Open
3xactCoder wants to merge 3 commits intomate-academy:masterfrom
3xactCoder:hash-map-task
Open

done hashmap task#1663
3xactCoder wants to merge 3 commits intomate-academy:masterfrom
3xactCoder:hash-map-task

Conversation

@3xactCoder
Copy link

No description provided.

Copy link

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

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 because there are several checklist violations and one important generics issue that should be fixed before this can be considered complete for the task. Functionally your MyHashMap API (put, getValue, getSize) and resize logic look good, but the assignment’s checklist is an explicit requirement, so these items are blocking.

Critical issues to fix:

  1. [CHECKLIST ITEM #1] Remove the redundant blank line immediately after the class MyHashMap<K, V> { declaration (start the class body without an empty line).
  2. [CHECKLIST ITEM #5] Move the private inner class Node<K, V> so that it appears after the public constructor and public API methods (put, getValue, getSize).
  3. [CHECKLIST ITEM #7] Add an explicit access modifier to the Node constructor, e.g. private Node(K key, V value), so all constructors/methods have access modifiers.
  4. [CHECKLIST ITEM #2] Replace the magic number 2 used in capacity *= 2; with a named constant such as private static final int RESIZE_MULTIPLIER = 2; and use that constant in resize().
  5. Fix the generic array allocation warnings by making the type explicit: in both the constructor and resize() use table = (Node<K, V>[]) new Node[capacity]; and annotate the field or constructor with @SuppressWarnings("unchecked") to acknowledge the cast.

Once you address these points, your solution will both meet the functional requirements and align with the checklist and Java conventions. You’re very close—your core logic is solid, and these adjustments will polish the implementation to the required standard.


✨ 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 +16 to +25
private static class Node<K, V> {
private K key;
private V value;
private Node<K, V> next;

Node(K key, V value) {
this.key = key;
this.value = value;
}
}

Choose a reason for hiding this comment

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

This violates checklist item #5: "Private methods and classes should be after public ones in your class." The private inner class Node<K, V> is declared before the public constructor and public API methods; move the private inner class declaration after public members. (Node declaration)

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

Node(K key, V value) {

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 is declared without an explicit access modifier (Node(K key, V value)). Add an explicit modifier (for example private Node(K key, V value)) so all constructors/methods have access modifiers.

}

private void resize() {
capacity *= 2;

Choose a reason for hiding this comment

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

This violates checklist item #2: "Any magic numbers should be constants... Move all hardcoded values to constant fields... For example: constants can be defaultCapacity, loadFactor..." The resize multiplier 2 is used directly in capacity *= 2;. Extract it to a named constant (e.g., RESIZE_MULTIPLIER) instead of using the literal.

this.capacity = DEFAULT_CAPACITY;
this.loadFactor = LOAD_FACTOR;
this.threshold = (int) (capacity * loadFactor);
this.table = new Node[capacity];

Choose a reason for hiding this comment

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

This addresses checklist item #8: "Be attentive with the diamond operator... What is the difference between: new Node<>(...) and new Node<K,V>(...)and new Node(...)." The code allocates a generic array with new Node[capacity] (constructor) which produces an unchecked raw-type array; consider using table = (Node<K, V>[]) new Node[capacity]; with @SuppressWarnings("unchecked") or another safe pattern to make the generics usage explicit and avoid warnings. (allocation site)

threshold = (int) (capacity * loadFactor);

Node<K, V>[] oldTable = table;
table = new Node[capacity];

Choose a reason for hiding this comment

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

Same generic-array allocation occurs in resize() with table = new Node[capacity];. Apply the same fix (cast with @SuppressWarnings("unchecked")) here as well to avoid unchecked-assignment warnings.

Copy link

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

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.

Copy link

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

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 because a few checklist-related issues are still present that are considered critical for this task, even though the main MyHashMap functionality is implemented correctly. The generic array creation currently uses raw new Node[capacity] in both the constructor and resize(), which leads to unchecked/generic-array warnings and violates the generics checklist; you should change these to table = (Node<K, V>[]) new Node[capacity]; and add @SuppressWarnings("unchecked") at the narrowest scope (e.g., on the constructor or method) to explicitly acknowledge the cast. The Node constructor is declared public, which breaks encapsulation for a private inner class and conflicts with the access-modifiers checklist; please make it private Node(K key, V value) (or at least non-public). The resize logic currently doubles capacity via a bit-shift capacity = capacity << 1;, which is effectively a hidden magic number; introduce a named constant like private static final int RESIZE_MULTIPLIER = 2; and use capacity *= RESIZE_MULTIPLIER; for clarity. Also, rename the typo-like variable olTable to something descriptive like oldTable or oldTableArray to align with the informative naming guideline. Once these are fixed, your implementation should both meet the functional requirements and satisfy the core checklist constraints—overall you’re very close, just need these polishing steps.


✨ 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() {
capacity = capacity << 1;

Choose a reason for hiding this comment

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

This uses a bit-shift to double capacity: capacity = capacity << 1;. This violates the checklist guidance about magic numbers: "Any magic numbers should be constants Your code should be easy to read. Move all hardcoded values to [constant fields] ... For example: constants can be defaultCapacity, loadFactor..." Replace the implicit multiplier/shift with a named constant such as private static final int RESIZE_MULTIPLIER = 2; and use capacity *= RESIZE_MULTIPLIER; for clarity and to satisfy the checklist.

this.capacity = DEFAULT_CAPACITY;
this.loadFactor = LOAD_FACTOR;
this.threshold = (int) (capacity * loadFactor);
this.table = new Node[capacity];

Choose a reason for hiding this comment

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

Generic array creation uses a raw type: this.table = new Node[capacity];. Per previous guidance this must be fixed to avoid unchecked/generic-array issues. Quote (previous guidance): "Fix the generic array allocation warnings by making the type explicit: in both the constructor and resize() use table = (Node<K, V>[]) new Node[capacity]; and annotate the field or constructor with @SuppressWarnings("unchecked") to acknowledge the cast." Apply that pattern in the constructor (this.table) and in resize() (table = ...) and add @SuppressWarnings("unchecked") where appropriate.

threshold = (int) (capacity * loadFactor);

Node<K, V>[] olTable = table;
table = new Node[capacity];

Choose a reason for hiding this comment

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

Same generic-array issue appears in resize(): table = new Node[capacity];. Use the explicit cast and @SuppressWarnings("unchecked") as explained above: table = (Node<K, V>[]) new Node[capacity]; to remove raw-type/unchecked warnings.

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

public Node(K key, V value) {

Choose a reason for hiding this comment

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

The Node constructor is declared public Node(K key, V value). This weakens encapsulation and violates the checklist guidance about access modifiers: "Remember about access modifiers. All fields and methods should have access modifiers. Remember about the encapsulation principle." Make the constructor non-public (e.g. private Node(K key, V value)) since Node is a private inner class.

capacity = capacity << 1;
threshold = (int) (capacity * loadFactor);

Node<K, V>[] olTable = table;

Choose a reason for hiding this comment

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

The variable name olTable is not descriptive and violates the checklist item: "Create informative variable and method names. ... Do not use abstract words like string or array as variable name. Do not use one-letter names. The name of the method should make it clear what it does." Rename olTable to something clearer such as oldTable or oldTableArray.

Copy link

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

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 this solution as an educational override to help you move forward, but there is one critical point from the last review that still needs your attention. You've done a great job addressing most of the previous feedback, such as using constants and fixing the Node constructor's access modifier. The main issue that remains is the incorrect creation of the generic array in both the MyHashMap constructor and the resize method. To fix this and ensure type safety, you must use an explicit cast: this.table = (Node<K, V>[]) new Node[capacity];. Remember to also add @SuppressWarnings("unchecked") to the constructor. Keep up the great work!


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

Footnotes

  1. Rate AI review example

this.capacity = DEFAULT_CAPACITY;
this.loadFactor = LOAD_FACTOR;
this.threshold = (int) (capacity * loadFactor);
this.table = new Node[capacity];

Choose a reason for hiding this comment

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

This was a key point from the last review. Creating a generic array requires an explicit cast. Please change this to this.table = (Node<K, V>[]) new Node[capacity]; and add @SuppressWarnings("unchecked") to the MyHashMap constructor.

threshold = (int) (capacity * loadFactor);

Node<K, V>[] oldTable = table;
table = new Node[capacity];

Choose a reason for hiding this comment

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

Similar to the constructor, the generic array creation here is incorrect. It should be table = (Node<K, V>[]) new Node[capacity];. The @SuppressWarnings("unchecked") on the method is correctly placed to handle the warning from this cast.

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