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 of the key-value #1674

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

Conversation

pMukuta
Copy link

@pMukuta pMukuta commented Jan 18, 2025

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.


Thank you for your hard work on this implementation! 😊 While your code meets many of the task requirements, there are a couple of critical issues that need to be addressed to ensure the functionality is robust and aligns with the expected behavior:

  1. Handling Duplicate null Keys: The put method currently allows multiple null keys to be stored. This could lead to unexpected behavior, as typically, a storage system should not allow duplicate keys. Please add a check to prevent storing duplicate null keys.

  2. Full Storage Handling: The put method does not currently handle the situation where the storage is full. This could lead to errors or data loss if new entries are added beyond the storage capacity. Consider implementing a mechanism to handle this scenario, such as throwing an exception or ignoring new entries when the storage is full.

Once these issues are resolved, your implementation will be much stronger and ready for approval. Keep up the great work, and don't hesitate to reach out if you have any questions or need further guidance! 🚀

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

Comment on lines 41 to 45
if (size < MAX_SIZE) {
keys[size] = key;
values[size] = value;
size++;
}

Choose a reason for hiding this comment

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

The put method does not handle the case when the storage is full and a new key-value pair is attempted to be added. Consider adding a condition to handle this scenario, such as throwing an exception or ignoring the new entry.

Comment on lines 22 to 32
if (key == null) {
for (int i = 0; i < size; i++) {
K targetKey = keys[i];
if (targetKey == null) {
values[i] = value;
return;
}
}
keys[size] = null;
values[size] = value;
size++;

Choose a reason for hiding this comment

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

When the key is null, the put method does not check for duplicate null keys. This could lead to multiple null keys being stored, which may not be the intended behavior. Consider adding a check to prevent duplicate null keys.

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.

Review Summary

Great job on implementing the StorageImpl class! 🎉 Your code meets the task requirements and constraints, effectively handling full storage by throwing an exception and using arrays without collections. Here are some key points and areas for improvement:

  • Handling Duplicate null Keys: You've made progress by checking for existing null keys and updating their values. However, ensure that no duplicate null keys are stored to prevent unexpected behavior.

  • Full Storage Handling: Excellent work on throwing a RuntimeException when the storage is full, which is a suitable approach for this scenario.

  • Code Structure: Your use of arrays, initialization in the constructor, and avoidance of magic numbers are commendable. The code is concise, with clear and informative naming.

While the implementation is correct and meets the specified requirements, addressing the issue with duplicate null keys will enhance the robustness of your solution. Keep up the great work, and continue refining your skills! 🚀

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

Comment on lines +25 to +32
if (key == null) {
for (int i = 0; i < size; i++) {
if (keys[i] == null) {
values[i] = value;
return;
}
}
keys[size] = null;

Choose a reason for hiding this comment

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

The current implementation allows multiple null keys to be stored. According to the task requirements, you should prevent storing duplicate null keys. Consider adding a check to ensure that a null key is not added if it already exists in the storage.

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