Skip to content
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.

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

Implemented the Storage #1669

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

Conversation

yaroslav-pryshchepa
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.

Great job on implementing the StorageImpl class! 🎉 Your code meets all the task requirements and constraints, and you've done well to avoid common mistakes. While there are some minor improvements suggested regarding object comparison using equals() instead of ==, these don't impact the core functionality. Keep up the good work and consider these suggestions for future enhancements! 😊

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

@Override
public void put(K key, V value) {
for (int i = 0; i < counter; i++) {
if (key == dataOfKeys[i]) {

Choose a reason for hiding this comment

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

Using == to compare objects is not recommended for non-primitive types. This should be replaced with equals() to properly compare the contents of the objects.

if (key == dataOfKeys[i]) {
dataOfValues[i] = value;
return;
} else if (key != null && key.equals(dataOfKeys[i])) {

Choose a reason for hiding this comment

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

This check for key.equals(dataOfKeys[i]) is correct, but the previous == check is redundant for non-primitive types. Consider removing the == check and using only equals() for object comparison.

}

@Override
public V get(K key) {
for (int i = 0; i < counter; i++) {
if (key == dataOfKeys[i]) {

Choose a reason for hiding this comment

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

Similar to the put method, using == for object comparison is not appropriate here. Use equals() instead to compare the key with dataOfKeys[i].

for (int i = 0; i < counter; i++) {
if (key == dataOfKeys[i]) {
return dataOfValues[i];
} else if (key != null && key.equals(dataOfKeys[i])) {

Choose a reason for hiding this comment

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

This equals() check is correct, but the == check above is redundant for non-primitive types. Consider removing the == check and using only equals() for object comparison.

public StorageImpl() {
dataOfKeys = (K[]) new Object[MAX_ITEMS_NUMBER];
dataOfValues = (V[]) new Object[MAX_ITEMS_NUMBER];
counter = 0;

Choose a reason for hiding this comment

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

Suggested change
counter = 0;

Comment on lines 7 to 9
private K[] dataOfKeys;
private V[] dataOfValues;
private int counter;

Choose a reason for hiding this comment

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

Suggested change
private K[] dataOfKeys;
private V[] dataOfValues;
private int counter;
private K[] keys;
private V[] values;
private int size;

@Override
public void put(K key, V value) {
for (int i = 0; i < counter; i++) {
if (key == dataOfKeys[i]) {

Choose a reason for hiding this comment

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

let's create separate method indexOf() to avoid duplication of code in put and get methods

@Override
public void put(K key, V value) {
if (indexOf(key) != -1) {

Choose a reason for hiding this comment

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

create local variable, don't need to call indexOf method twice

}

@Override
public V get(K key) {
if (indexOf(key) != -1) {

Choose a reason for hiding this comment

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

same here

Comment on lines 43 to 47
if (key == keys[i]) {
return i;
} else if (key != null && key.equals(keys[i])) {
return i;
}

Choose a reason for hiding this comment

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

Suggested change
if (key == keys[i]) {
return i;
} else if (key != null && key.equals(keys[i])) {
return i;
}
if (key == keys[i] || key != null && key.equals(keys[i])) {
return i;
}

Copy link
Contributor

@liliia-ponomarenko liliia-ponomarenko left a comment

Choose a reason for hiding this comment

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

Awesome 😎

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.

4 participants