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

solution #1350

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

solution #1350

wants to merge 3 commits into from

Conversation

DojiDoe
Copy link

@DojiDoe DojiDoe commented Nov 23, 2023

No description provided.

Copy link
Contributor

@Rommelua Rommelua left a comment

Choose a reason for hiding this comment

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

Before you create PR with your homework, you need to go through the checklist under the task and correct all the points described there. The mentor will not check the work until the checklist points are corrected

Comment on lines +20 to +49
@Override
public boolean equals(Object storageObject) {
if (storageObject == this) {
return true;
}
if (storageObject == null) {
return false;
}
if (storageObject.getClass().equals(StorageObject.class)) {
StorageObject current = (StorageObject) storageObject;
boolean keyEquals = (this.key == null && current.key == null)
|| (this.key != null && this.key.equals(current.key));
boolean valueEquals = (this.value == null && current.value == null)
|| (this.value != null && this.value.equals(current.value));
return keyEquals && valueEquals;
}
return false;
}

@Override
public int hashCode() {
int result = 17;
result = 31 * result + (key == null ? 0 : key.hashCode());
result = 31 * result + (value == null ? 0 : value.hashCode());
return result;
}

@Override
public String toString() {
return "{" + "key="
Copy link
Contributor

Choose a reason for hiding this comment

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

delete this

Copy link
Author

Choose a reason for hiding this comment

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

delete this

why? what is the other solution?

Choose a reason for hiding this comment

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

make it nested class

it uses only inside `Storage, so we don't need it public


public class StorageImpl<K, V> implements Storage<K, V> {

private final StorageObject[] storage = new StorageObject[10];
Copy link
Contributor

Choose a reason for hiding this comment

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

image
image


private final StorageObject[] storage = new StorageObject[10];

private int size = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private int size = 0;
private int size;

@DojiDoe DojiDoe requested a review from Rommelua November 24, 2023 13:15
Comment on lines +20 to +49
@Override
public boolean equals(Object storageObject) {
if (storageObject == this) {
return true;
}
if (storageObject == null) {
return false;
}
if (storageObject.getClass().equals(StorageObject.class)) {
StorageObject current = (StorageObject) storageObject;
boolean keyEquals = (this.key == null && current.key == null)
|| (this.key != null && this.key.equals(current.key));
boolean valueEquals = (this.value == null && current.value == null)
|| (this.value != null && this.value.equals(current.value));
return keyEquals && valueEquals;
}
return false;
}

@Override
public int hashCode() {
int result = 17;
result = 31 * result + (key == null ? 0 : key.hashCode());
result = 31 * result + (value == null ? 0 : value.hashCode());
return result;
}

@Override
public String toString() {
return "{" + "key="

Choose a reason for hiding this comment

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

make it nested class

it uses only inside `Storage, so we don't need it public

Comment on lines +9 to +14
private static final int PRIMARY_SIZE = 0;
private final StorageObject[] storage;
private int size;

public StorageImpl() {
this.size = PRIMARY_SIZE;

Choose a reason for hiding this comment

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

redundant, java set 0 as default for primitive

Suggested change
private static final int PRIMARY_SIZE = 0;
private final StorageObject[] storage;
private int size;
public StorageImpl() {
this.size = PRIMARY_SIZE;
private final StorageObject[] storage;
private int size;
public StorageImpl() {

Comment on lines 43 to 50
if (object == null) {
break;
} else if (object.getKey() == null && key == null) {
return (V) object.getValue();
} else if (object.getKey() != null
&& object.getKey().equals(key)) {
return (V) object.getValue();
}

Choose a reason for hiding this comment

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

if statement could be simplified.

you have similar logic in each clause

Comment on lines 24 to 35
storage[i] = storageToAdd;
size++;
break;
} else if (storage[i].getKey() == null
&& storageToAdd.getKey() == null) {
storage[i] = storageToAdd;
break;
} else if (storage[i].getKey() != null
&& storage[i].getKey().equals(storageToAdd.getKey())) {
storage[i] = storageToAdd;
break;
}

Choose a reason for hiding this comment

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

simplify if-else

@DojiDoe DojiDoe requested a review from sokimaaa December 7, 2023 14:14
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.

3 participants