-
Notifications
You must be signed in to change notification settings - Fork 231
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
[CS2113-T12-3] BudgetBuddy #35
base: master
Are you sure you want to change the base?
[CS2113-T12-3] BudgetBuddy #35
Conversation
Added more JUnit Tests
…yao-defensiveCoding Bug fixes for unsupported commands - index for mark, unmark and remov…
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.
Generally great job done! The explanantions are very clear and detailed, various types of diagrams are provided. But there are some repetitive diagrams as well as unnecessary details provided.
docs/DeveloperGuide.md
Outdated
|
||
<img alt="ClassDiagram2.png" height="800" src="ClassDiagram2.png" width="700"/> | ||
|
||
#### Activity diagram |
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.
Good job including various types of diagrams for illustration!
docs/DeveloperGuide.md
Outdated
#### Class diagram | ||
The class diagram below outlines the relationships between the classes involved in the Budget Management feature: | ||
|
||
<img alt="ClassDiagram2.png" height="800" src="ClassDiagram2.png" width="700"/> |
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 the green dots in front of the methods be positive or negative signs instead?
| listSavings() | void | Prints the savings, filtered by `filterCategory`, to the CLI | | ||
| calculateRemainingSavings() | double | Calculates the remaining amount after deducting total expenses | | ||
|
||
The Listing Savings feature follows these steps when a user inputs a command to list savings: |
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.
is this part of explanations truly 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.
same for all other parts that include these steps
|---------------|-------------|-------------------------------------------------------------------------------------------| | ||
| editExpense() | void | Edits the `category`, `amount` and `description` for the expense in the specified `index` | | ||
|
||
The following UML Sequence diagram below shows how the Edit Expense Feature Command is executed when a user |
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 sequence diagrams seem to be repetitive
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.
I have some comments for you! Overall, good job! Keep it up!
an `ExpenseList` object, `category`, `index`, `amount` and `description`. The relevance of these Class Attributes in | ||
`EditExpenseCommand` is as follows: | ||
|
||
| Class Attribute | Variable Type | Relevance | |
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.
Good display of the class attributes and variable type in a table format
After determining the type of `CommandCreator` object, the Parser initializes the `CommandCreator` object | ||
with all its required parameters. | ||
|
||
Here are some examples : |
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.
Great work in giving examples of how your parser works!
| exchangeRates | Map<Currency, Double> | Stores exchange rates with currencies as keys | | ||
|
||
When `BudgetBuddy` calls `command.execute()`, `ChangeCurrencyCommand` employs the following methods from `CurrencyConverter` to convert the currency of all financial records: | ||
|
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.
Good Consistency with making the tables for the class attributes
docs/DeveloperGuide.md
Outdated
| setDefaultCurrency | void | Updates the default currency to a new value | | ||
|
||
|
||
Here's the step-by-step process when the user uses the Currency Converter feature: |
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.
I think since you have the sequence diagrams, I feel it is not necessary to explain your commands again
docs/DeveloperGuide.md
Outdated
class. Below is a description of the key class attributes and methods involved in the budget setting and listing | ||
process: | ||
|
||
##### Class Attributes for `SetBudgetCommand`: |
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.
Formatting for these tables on the DG is hard to see.
Add Comments to Recurring Expenses Feature
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.
Overall, extremely detailed and informative. Awesome work!
docs/DeveloperGuide.md
Outdated
|-----------------------------|---------------|--------------------------------------------------------------------| | ||
| setBudget(category, budget) | void | Sets or updates the budget for a given category in the ExpenseList | | ||
| getBudgets() | List<Budget> | Retrieves the list of all budgets set | | ||
|
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 three tables above are not properly rendered in the result html. Checking the formatting may help.
docs/DeveloperGuide.md
Outdated
### 3.1 Architecture | ||
The following diagram provides a rough overview of how BudgetBuddy is built | ||
|
||
![Diagram of overview of BudgetBuddy](diagrams/Introduction.jpg) |
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.
Very clear and understandable architecture. Good job!
docs/DeveloperGuide.md
Outdated
#### Class diagram | ||
The class diagram below outlines the relationships between the classes involved in the Budget Management feature: | ||
|
||
<img alt="ClassDiagram2.png" height="800" src="ClassDiagram2.png" width="700"/> |
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 green dots and red square doesn't follow the standard + - notation for public/private members. Maybe using the standard or further explanations can help.
docs/DeveloperGuide.md
Outdated
where the command gets executed : | ||
![UML Sequence Diagram of Command](diagrams/sequence_diagram_command.jpg) | ||
|
||
#### 3.5 Storage Class |
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 Storage class can use some further information for developers. Information such as how the save/load function is implemented, how the data is formatted in the output files when writing and how the data is interpreted when reading.
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.
All in all the team has done a marvellous job in documenting DG! It is a comprehensive and detailed breakdown of their system architecture with interactions between classes well explained. Further improvement suggestions could be to have a consistent standard with the UML diagrams (background colour, activation bar etc), such that the presentation is more coherent.
docs/DeveloperGuide.md
Outdated
process: | ||
|
||
##### Class Attributes for `SetBudgetCommand`: | ||
| Class Attribute | Variable Type | Relevance | |
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.
Rendering errors for the command table in HTML view. Recommend checking the formatting for the tables again.
docs/DeveloperGuide.md
Outdated
The following UML Sequence diagram shows how `SetBudgetCommand` works when a user sets a budget for a category in the | ||
following format: `set budget c/<Category> b/<Amount>` | ||
|
||
<img alt="SetBSD.png" height="400" src="SetBSD.png" width="700"/> |
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.
A clear and concise drawing of the UML diagram! However, would further recommend to standardise all return commands to use dotted arrows.
docs/DeveloperGuide.md
Outdated
by what amount, making it easy for users to identify areas of concern. | ||
|
||
|
||
#### Sequence diagrams |
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.
A minor issue with the consistency of the sequence diagrams: The diagram here is a concise drawing of UML while the later ones utilise the full annotation. Would it be possible for such standards to be consistent (e.g. Available drawing of activation bar for all of them)?
docs/DeveloperGuide.md
Outdated
The following UML Sequence Diagram shows the processes of the MenuCommand upon the call of its execute() command: | ||
![Sequence Diagram for Menu Command](diagrams/sequenceDiagram_MenuCommand.jpg) | ||
|
||
Given below is an example usage scenario and how the full Menu feature works : |
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.
A good and detailed breakdown of how the feature functions!
docs/DeveloperGuide.md
Outdated
| addSaving() | void | Add savings to the existing list of `savings` | | ||
|
||
The following UML Sequence diagram shows how the Parser works to obtain the relevant inputs for the Add Expense Feature : | ||
![Sequence Diagram for Parser for Add Expense Feature](docs\diagram\sequenceDiagram_AddSavings.png) |
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.
A minor issue: this UML diagram not rendering correctly.
…Update Update JarFile link in UserGuide.md
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.
Overall, the DG is detailed and well-explained. A few minor issues such as format and notations used can be improved. Great job!
docs/DeveloperGuide.md
Outdated
##### Class Attributes for `SetBudgetCommand`: | ||
| Class Attribute | Variable Type | Relevance | | ||
|-----------------|---------------|---------------------------------------------------------------------| | ||
| expenseList | ExpenseList | Object containing the list of expenses to check against set budgets | | ||
| category | String | The category for which the budget is being set | | ||
| budget | double | The budget amount to be set for the category | | ||
|
||
##### Class Attributes for `ListBudgetCommand`: | ||
| Class Attribute | Variable Type | Relevance | | ||
|-----------------|---------------|---------------------------------------------------------------------| | ||
| expenseList | ExpenseList | Object containing the list of expenses to check against set budgets | | ||
|
||
|
||
Upon the call of the `execute()` method in `BudgetBuddy` using `command.execute()`, `SetBudgetCommand` will update the | ||
budget in `ExpenseList` using `setBudget`. Similarly, `ListBudgetCommand` will fetch and display all categories with | ||
their budgets using `getBudgets`, and highlight those that are above the set budget. | ||
|
||
##### Key Methods used from `ExpenseList` | ||
| Method | Return Type | Relevance | | ||
|-----------------------------|---------------|--------------------------------------------------------------------| | ||
| setBudget(category, budget) | void | Sets or updates the budget for a given category in the ExpenseList | | ||
| getBudgets() | List<Budget> | Retrieves the list of all budgets set | |
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.
It is difficult to understand the tables under the class attributes for SetBudgetCommand
, ListBudgetCommand
and ExpenseList
. The formatting here can definitely be improved.
docs/DeveloperGuide.md
Outdated
#### Class diagram | ||
The class diagram below outlines the relationships between the classes involved in the Budget Management feature: | ||
|
||
<img alt="ClassDiagram2.png" height="800" src="ClassDiagram2.png" width="700"/> |
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 class diagram here doesn't follow the standard used in the course website. The access modifiers are supposed to be denoted using + for public and - for private members.
docs/DeveloperGuide.md
Outdated
|
||
The following UML Sequence Diagram depicts the process of the process through which an input is gone through the application, up till the point | ||
where the command gets executed : | ||
![UML Sequence Diagram of Command](diagrams/sequence_diagram_command.jpg) |
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.
Good job on explaining how the command is processed and executed! The sequence diagram illustrated is very clear
docs/DeveloperGuide.md
Outdated
The activity diagram provides an overview of the Budget Management feature's workflow: | ||
|
||
<img alt="ActivityDiagram.png" height="600" src="ActivityDiagram.png" width="700"/> |
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 workflow of Budget Management feature has been well explained here using the activity diagram, kudos!
Include images for Expected Outputs
Remove repetitive explanation of features in PPP
Add changes to reduce savings UI
Update the addsaving function
Add Final Changes to DG, UG and Add minor bug-fixes
Minor Changes to DG, UG, Storage and RecurringExpenseCommandCreator
Add disclaimer to the running of JAR file
Update PPP
Add updates to PPP
BudgetBuddy is for users who wish to handle and track their expenses on a singular platform. It provides a faster and more efficient way to track and calculate expenses, and the ability to deal with finances on a singular platform.