Skip to content

First solution by Viodon #1914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Viodon wants to merge 3 commits into
base: master
Choose a base branch
from
Open

First solution by Viodon #1914

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
286d204
First solution
Viodon May 10, 2026
7eb1523
added fixed after review
Viodon May 10, 2026
51a9550
added fixed after review v2
Viodon May 11, 2026
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
39 changes: 38 additions & 1 deletion src/main/java/core/basesyntax/impl/StorageImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,54 @@
import core.basesyntax.Storage;

public class StorageImpl<K, V> implements Storage<K, V> {
public static final int MAX_SIZE = 10;
private Object[] keys;
private Object[] values;

public StorageImpl() {
keys = new Object[MAX_SIZE];
values = new Object[MAX_SIZE];
}

@Override
public void put(K key, V value) {
for (int i = 0; i < keys.length; i++) {
if (keys[i] != null && keys[i].equals(key) || keys[i] == key) {
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 10, 2026

Choose a reason for hiding this comment

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

The reference equality check keys[i] == key is redundant. The condition keys[i].equals(key) already handles all cases including when both are null, so this OR clause can be removed.

Copy link
Copy Markdown
Author

@Viodon Viodon May 11, 2026

Choose a reason for hiding this comment

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

@mateacademy-ai-mentor строка потрібна, ввона не обробляє варіанти коли ключ = null, без неї код працює не коректно

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 11, 2026

Choose a reason for hiding this comment

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

Redundant check: The || keys[i] == key part is unnecessary. keys[i].equals(key) handles null keys correctly (returns false when keys[i] is null), so the reference equality check for null is already covered. Keep only the keys[i] != null && keys[i].equals(key) condition.

values[i] = value;
return;
}

if (values[i] == null) {
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 10, 2026

Choose a reason for hiding this comment

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

The empty slot detection checks values[i] == null, but the task requirement specifies it should check only keys[i] == null. Change to maintain consistency with the stated requirement.

Copy link
Copy Markdown
Author

@Viodon Viodon May 11, 2026

Choose a reason for hiding this comment

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

Шукає вільний слот для значення коли ключ не збігається з жодним іншим, перевіряємо values[і], перевіряємо values[i], тому що може бути значення з ключем null, @mateacademy-ai-mentor

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 11, 2026

Choose a reason for hiding this comment

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

CRITICAL BUG: Empty slot detection checks values[i] == null instead of keys[i] == null. This breaks when storing null values—they'll be treated as empty slots and overwritten. Change to if (keys[i] == null) to properly detect unused positions.

values[i] = value;
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 11, 2026

Choose a reason for hiding this comment

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

CRITICAL BUG: After fixing the empty slot condition, this line should set keys[i] = key first, then values[i] = value. Currently if the fix isn't applied, this assigns value before the key.

keys[i] = key;
return;
}

}
}

@Override
public V get(K key) {
for (int i = 0; i < keys.length; i++) {
if (keys[i] != null && keys[i].equals(key)) {
return (V) values[i];
}
if (keys[i] == key) {
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 11, 2026

Choose a reason for hiding this comment

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

Redundant check: Same issue as in put() - keys[i].equals(key) already handles null key comparison. Remove the separate keys[i] == key check.

return (V) values[i];
Comment on lines +39 to +41
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 10, 2026

Choose a reason for hiding this comment

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

This keys[i] == key check is redundant - the first condition on line 34 already handles null keys and equal keys via keys[i].equals(key). This duplicate check should be removed.

Copy link
Copy Markdown
Author

@Viodon Viodon May 11, 2026

Choose a reason for hiding this comment

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

@mateacademy-ai-mentor ця стрічка потрібна, вона перевіряє ключ null, в рядку 34 значення порівнюються лише якщо keys[i] != null, щоб уникнути помилки NullPointerExceptions

Comment on lines +36 to +41
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor May 11, 2026

Choose a reason for hiding this comment

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

DRY violation: The return (V) values[i] logic is duplicated in get() (lines 37 and 40). Consider extracting the index-finding logic to a private method findKeyIndex(K key) that returns the index or -1, then use it in both get() and put().

}
}
return null;
}

@Override
public int size() {
return -1;
int size = 0;
for (int i = 0; i < values.length; i++) {
if (values[i] != null) {
size++;
}
}
return size;
}

}
Loading