Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 69 additions & 1 deletion src/main/java/core/basesyntax/MyHashMap.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,87 @@
package core.basesyntax;

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.' There should be no empty lines between the package declaration and class declaration.

public class MyHashMap<K, V> implements MyMap<K, V> {
Comment on lines 1 to 2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add empty line after package declaration.

private Node<K, V>[] table;
private int size;
private static final int DEFAULT_CAPACITY = 16;
private static final float LOAD_FACTOR = 0.75f;


public MyHashMap() {
table = new Node[DEFAULT_CAPACITY];
size = 0;
}

private void checkThreshold() {
if (size >= table.length * LOAD_FACTOR) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Resize is triggered after adding a new element, currently, the logic is:

size++;
checkThreshold();

This means that the map first adds the new element and only then checks whether resizing is needed.

This is not necessarily a critical bug, but the current condition:

if (size >= table.length * LOAD_FACTOR)

means that for the default capacity 16 and load factor 0.75, resizing happens immediately after adding the 12th element.

Usually, it is slightly clearer to resize only when the threshold is exceeded:

if (size > table.length * LOAD_FACTOR) {
    resize();
}

or to check before adding the new element:

if (size + 1 > table.length * LOAD_FACTOR) {
    resize();
}

The current solution can work, but this small change would make the behavior more natural.

resize();
}
}

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

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 #2: 'Any magic numbers should be constants'. The value 2 (for doubling capacity) should be declared as a named constant (e.g., CAPACITY_MULTIPLIER) for better code readability.

size = 0;

for (Node<K, V> node : oldTable) {
while (node != null) {
put(node.key, node.value);
node = node.next;
}
}
}
Comment on lines +14 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the checklist:

Private methods and classes should be after public ones in your class.

Currently, checkThreshold() and resize() are placed before the public methods put(), getValue(), and getSize().


@Override
public void put(K key, V value) {
int hash = key == null ? 0 : key.hashCode();
int index = hash & (table.length - 1);

Node<K, V> current = table[index];
while (current != null) {
if ((current.key == null && key == null) || (current.key != null && current.key.equals(key))) {
current.value = value;
return;
}
current = current.next;
}

Node<K, V> newNode = new Node<>(key, value);
newNode.hash = hash;
newNode.next = table[index];
table[index] = newNode;
size++;
checkThreshold();
}

@Override
public V getValue(K key) {
int hash = key == null ? 0 : key.hashCode();
int index = hash & (table.length - 1);

Node<K, V> current = table[index];
while (current != null) {
if ((current.key == null && key == null) || (current.key != null && current.key.equals(key))) {
return current.value;
}
current = current.next;
}
return null;
}

@Override
public int getSize() {
return 0;
return size;
}

private static class Node<K, V> {
private final K key;
private V value;
private int hash;

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 hash field in Node is redundant, in put(), you assign:

newNode.hash = hash;

but this field is not used anywhere later.

In Node, you currently have something like:

private int hash;

This field can be removed because it does not bring any value to the current implementation.

private Node<K, V> next;

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