Skip to content

Add Storage key-value generic implementation#1912

Open
DanyilShkodiuk wants to merge 3 commits into
mate-academy:masterfrom
DanyilShkodiuk:jv-storage
Open

Add Storage key-value generic implementation#1912
DanyilShkodiuk wants to merge 3 commits into
mate-academy:masterfrom
DanyilShkodiuk:jv-storage

Conversation

@DanyilShkodiuk
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@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

Your implementation is functionally correct and meets most requirements, including proper generics support, key-value storage with two arrays, key replacement logic, and the 10-element limit. However, CHECKLIST ITEM #5 is violated: the key comparison logic key == keys[i] || key != null && key.equals(keys[i]) is duplicated in both put() and get() methods. According to the DRY principle specified in the checklist, this repeated logic must be extracted into a private helper method like findKeyIndex(K key) that returns the index of the key or -1 if not found. This is required for maintaining the code and avoiding inconsistencies when the comparison logic needs to be updated. Please refactor the duplicated key comparison into a helper method and use it in both put() and get().


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

Important

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

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/main/java/core/basesyntax/impl/StorageImpl.java Outdated
Comment thread src/main/java/core/basesyntax/impl/StorageImpl.java Outdated
Copy link
Copy Markdown

@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

Your solution is approved. All task requirements have been successfully implemented: the generic Storage<K, V> interface works correctly with proper type parameters, the two-array storage structure enforces the 10-element limit via the MAX_SIZE constant, key comparison logic properly uses == and .equals(), and the put() and get() methods handle key replacement and retrieval as expected. The previous DRY principle violation has been properly resolved by extracting the key search logic into the findKeyIndex(K key) helper method, which is now used by both methods. No critical issues or blockers remain, and your implementation satisfies all checklist items.


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

Footnotes

  1. Rate AI review example

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