-
Notifications
You must be signed in to change notification settings - Fork 4
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
[mid v1.2] Add more validation to UpdateCommand #77
Conversation
|
||
if (batchToUpdate != null) { | ||
quantity -= batchToUpdate.getQuantity().getNumericValue(); | ||
} | ||
quantity += updatedBatch.getQuantity().getNumericValue(); | ||
|
||
if (quantity > 999999999) { |
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.
Use Java's Integer.MAX_VALUE
if the intention is to prevent integer overflow.
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.
You're right! Thanks for pointing it out. I will amend it by next PR.
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.
Probably want to use Java's Integer.MAX_VALUE to check for overflows instead of hardcoding it, otherwise it looks good!
Add max quantity to prevent integer overflow.
Add check to UpdateCommand to prevent removing medicine that does not exist.
Improve performance of getNewMedicineExpiry for UpdateCommand class to O(1) most of the time
List stays on current view for more convenient updating after using UpdateCommand.
Improve and add JUnit tests for UpdateCommand
Also removed Address Book Mode and Kevin's part of the user guides to be replaced with more coherent features in future update of UseGuide.