Skip to content

Added Hash Map Implementation for java#593

Open
bhaweshgit06 wants to merge 1 commit into
vichitr:masterfrom
bhaweshgit06:master
Open

Added Hash Map Implementation for java#593
bhaweshgit06 wants to merge 1 commit into
vichitr:masterfrom
bhaweshgit06:master

Conversation

@bhaweshgit06

Copy link
Copy Markdown

No description provided.

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

Thanks for the contribution! ,I left a couple of inline comments around edge cases (negative hash index and remove handling). Overall, nice work 👍

Comment thread DS/Java/HashMap.java

private int HashCode(K key) {
int hc = key.hashCode();
int bucketIndex = hc % numBucket;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bucket index can become negative:
In HashCode(), hc % numBucket may return a negative value if hashCode() is negative, which can cause buckets.get(bucketIndex) to crash.
Suggestion: use Math.floorMod(hc, numBucket) for a safe non-negative index.

Comment thread DS/Java/HashMap.java
Comment on lines +43 to +44
if (head.key.equals(key)) {
head.value = 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.

Potential NPE and return value issue in removeKey():

  • If the bucket is empty, head can be null, so head.key.equals(key) would throw an exception.

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