Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Hw fruit shop #1231
base: master
Are you sure you want to change the base?
Hw fruit shop #1231
Changes from all commits
c047a65
8d8bbfd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
getAll
method usesMap.copyOf(Storage.storage).entrySet()
, which creates a copy of the storage map. This might be unnecessary if you only need to read the entries. Consider returningStorage.storage.entrySet()
directly if immutability is not a concern, or useCollections.unmodifiableSet
to return an unmodifiable view if needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a static
HashMap
for storage can lead to concurrency issues if accessed by multiple threads simultaneously. Consider using a thread-safe collection likeConcurrentHashMap
if your application will be multi-threaded. Additionally, static fields can make unit testing more challenging, as they maintain state across tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
getOperation
method throws anIllegalArgumentException
if no matching operation is found. This is a good practice for handling invalid inputs. Ensure that this exception is handled appropriately wherever this method is used to prevent unexpected crashes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the
create
method, you are callingstorageDao.getQuantity(e)
inside theforEach
loop. This can be inefficient ifgetQuantity
involves complex operations or if the storage is large. Since you already have the entries fromgetAll()
, consider usingMap.Entry::getValue
to directly access the quantity, which would be more efficient.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loop starts from index 1, which assumes that the first line of the data is a header. Ensure that this assumption is documented or handled elsewhere if the input data format changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
getOperation
method is called withparts[0].trim().toLowerCase()
. Ensure that the operation letters in the input data are consistently formatted in lowercase to avoid unexpected errors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching
IllegalArgumentException
and rethrowing it as aRuntimeException
with a custom message is a good practice for providing more context. Ensure that this exception handling is consistent throughout the application.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code checks if the
handler
isnull
and throws anIllegalArgumentException
if no handler is found for a given operation. This is a good practice for handling unexpected cases. Ensure that theOperationStrategy
is correctly set up to provide handlers for all expected operations to avoid this exception.