STORAGE: implemented storage #1605
Conversation
There was a problem hiding this comment.
Hey there! It looks like there are a few important areas that need a bit of work before we can give the green light. Specifically, be careful with unchecked casts and potential ClassCastExceptions, and make sure to check array bounds to prevent ArrayIndexOutOfBoundsExceptions. Also, it's a good idea to avoid redundant code and use constants instead of magic numbers to make your code clearer and easier to maintain. Keep at it, and with a few tweaks, your code will be in great shape! 🛠️👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
There was a problem hiding this comment.
Hey there! It looks like we've got a few kinks to iron out before we can give this PR the green light. 🛠️ There's a potential bug that could cause some key-value pairs to get overwritten, which is definitely something we want to avoid. Also, I noticed a little bit of repeated code that could be cleaned up for a more polished look. No worries, though! With a bit of tweaking and a couple of fixes, we'll be on track for approval. Keep up the good work, and let's make this code shine! 💪😄
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
There was a problem hiding this comment.
Great job on submitting your Pull Request! 🌟 There are a few issues to address in the future: consider initializing 'index' in the constructor, use more specific exceptions with clear messages, refactor duplicated null key checks, replace magic numbers with array lengths, extract repeating logic to private methods, and initialize non-constant fields in the constructor. Keep up the good work and use these tips to enhance your code! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| public static final int INITIAL_SIZE = 10; | ||
|
|
||
| private K[] keys = (K[]) new Object[INITIAL_SIZE]; |
There was a problem hiding this comment.
Non-constant fields should be initialized in the constructor. Move the initialization of 'keys' and 'values' arrays to the constructor.
| if (key == null && keys[i] == null) { | ||
| if (values[i] == null) { | ||
| values[i] = value; | ||
| index++; | ||
| return; | ||
| } else { | ||
| values[i] = value; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Don't create repeating code. The logic for setting the value in 'values' array repeats. You can extract this logic to a private method to avoid duplication.
| if (index > INITIAL_SIZE - 1) { | ||
| throw new RuntimeException("Too many!"); |
There was a problem hiding this comment.
Using magic numbers directly in the code can be confusing. Consider using 'keys.length' instead of 'INITIAL_SIZE - 1' to check for the array bounds as it reflects the intent more clearly and is less error-prone.
| if (key == null && keys[i] == null) { | ||
| if (values[i] == null) { | ||
| values[i] = value; | ||
| index++; | ||
| return; | ||
| } else { | ||
| values[i] = value; | ||
| return; | ||
| } |
There was a problem hiding this comment.
The check for null key is duplicated in the 'put' and 'get' methods. Consider refactoring this into a private method to determine the index of a key, which can then be used in both 'put' and 'get' methods.
| @Override | ||
| public void put(K key, V value) { | ||
| if (index > INITIAL_SIZE - 1) { | ||
| throw new RuntimeException("Too many!"); |
There was a problem hiding this comment.
Throwing a generic RuntimeException with a vague message is not very informative. It's better to create a custom exception or use a more specific exception type with a clear message indicating the storage is full.
|
|
||
| private K[] keys = (K[]) new Object[INITIAL_SIZE]; | ||
| private V[] values = (V[]) new Object[INITIAL_SIZE]; | ||
| private int index = 0; |
There was a problem hiding this comment.
The 'index' field is not a constant but it's not initialized in the constructor. Consider initializing 'index' in the constructor for consistency and to follow the best practices.
No description provided.