-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add Budget class for Buyer class #79
Add Budget class for Buyer class #79
Conversation
Addition of the Budget class is necessary to represent the budget attribute for Buyer objects. Implemented the Budget class to encapsulate the budget information for buyers, allowing for better organization and management of budget-related data. Using a separate class for budget ensures separation of concerns and follows the single responsibility principle, improving code readability and maintainability. This commit also includes necessary adjustments to integrate the Budget class with the Buyer class.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #79 +/- ##
============================================
+ Coverage 72.43% 72.54% +0.10%
- Complexity 501 515 +14
============================================
Files 86 87 +1
Lines 1698 1748 +50
Branches 173 178 +5
============================================
+ Hits 1230 1268 +38
- Misses 422 430 +8
- Partials 46 50 +4 ☔ View full report in Codecov by Sentry. |
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.
Looks good to me
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.
LGTM, just some comments to take note of.
@@ -16,4 +16,5 @@ public class CliSyntax { | |||
public static final Prefix PREFIX_UNITNUMBER = new Prefix("unitNo/"); | |||
public static final Prefix PREFIX_BLOCK = new Prefix("blk/"); | |||
public static final Prefix PREFIX_POSTALCODE = new Prefix("postal/"); | |||
public static final Prefix PREFIX_BUDGET = new Prefix("b/"); |
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.
Do you think it is better to explicitly name the prefix as budget/ ?
Not really a big issue, just a personal preference.
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.
Should be fine I feel, like name is also "n/". We can discuss during meeting.
* Returns true if a given string is a valid budget amount. | ||
*/ | ||
public static boolean isValidBudget(String test) { | ||
return test.matches(VALIDATION_REGEX) && Double.parseDouble(test) >= 0; |
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.
Can a budget be $0? And is there a minimum and maximum a budget can be?
Things to consider, depends on our team requirement (need to ask them).
Addition of the Budget class is necessary to represent the budget attribute for Buyer objects.
Implemented the Budget class to encapsulate the budget information for buyers, allowing for better organization and management of budget-related data.
Using a separate class for budget ensures separation of concerns and follows the single responsibility principle, improving code readability and maintainability.
This commit also includes necessary adjustments to integrate the Budget class with the Buyer class.