-
Notifications
You must be signed in to change notification settings - Fork 248
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-1] ChChing #24
base: master
Are you sure you want to change the base?
[CS2113-T12-1] ChChing #24
Conversation
add Account 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 DG is incomplete. Non-Functional Requirements and Instructions for manual testing are not completed.
It may be better to focus on other classes rather than just all the Command classes. Storage, UI and Parser could be explained in the DG as well.
The main class in our program is the ```Record``` and ```RecordList``` abstract classes, in which ```Income```, ```Expense``` will inherit from ```Record``` and ```IncomeList``` and ```ExpenseList``` will inherit from ```RecordList```. Most commands will act on instances of the ```Income```, ```Expense```,```IncomeList``` and ```ExpenseList``` classes. | ||
|
||
![Record Class](images/Record_RecordList_UML_class.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.
For this UML diagram:
- Abstract class for RecordList and Record is incorrect. Instead of << >> , it should be denoted as {RecordList} instead?
- The font size of the diagram is a little bit small, making it blurry and hard to read. Perhaps this could be changed to make it easier to read.
<br> ![edit income sequence diagram](images/EditIncomeCommand_sequence_diagram.png) | ||
|
||
The edit expense command works in a similar way, with its sequence diagram as shown: | ||
<br> ![edit expense sequence diagram](images/EditExpenseCommand_sequence_diagram.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.
This edit expense sequence diagram may not be necessary due to its similarity to the edit income sequence diagram shown above. On the other hand, this font is clearer and more comprehensive to read as compared to the above sequence diagrams.
is called, which iterates through the expenseList, ```expenses``` and prints the index as well as a completed string of | ||
expenses in ```expenses```. | ||
|
||
![ListExpenseCommand](images/ListExpenseCommand_Sequence_Diagram.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.
For this diagram:
- This image is named as a sequence diagram when it is actually a class diagram
- Other than showing ListExpenseCommand class inherits from Command class, this does not add much significance at this point of the dg
- Possible to move this class diagram up above
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 execute portion of the class diagram is also incorrect. It should be something like execute( ... ): void.
Afterwards, the ```execute()``` method will call the ```printSelector()``` method from ```Selector```. | ||
The ```printSelector()``` method will print all the available currencies in the selector hashmap. | ||
The selected currencies will be marked with a ```[X]``` and the unselected currencies will be marked with a ```[ ]```. | ||
|
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 in the DG is a little bit inconsistent. While ListExpenseCommand and EditIncomeCommand uses numbering to sequence the explanation, the other Commands are explained in one large chunk. It may be better to standardize the way of explaining.
### Record and RecordList | ||
|
||
The main class in our program is the ```Record``` and ```RecordList``` abstract classes, in which ```Income```, ```Expense``` will inherit from ```Record``` and ```IncomeList``` and ```ExpenseList``` will inherit from ```RecordList```. Most commands will act on instances of the ```Income```, ```Expense```,```IncomeList``` and ```ExpenseList``` classes. | ||
|
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 notation for abstract class for Record and RecordList is wrong. It should be {Abstract} rather than << Abstract >>.
Overall, a good detailed class diagram provided
### Target and TargetStorage | ||
|
||
The `Target` and `TargetStorage` class allows users to set a target for their ideal balance. | ||
|
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 use of protected variables as seen from the # notation.
@@ -8,23 +8,128 @@ | |||
|
|||
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.} | |||
|
|||
## Implementation | |||
|
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 could add the system architecture diagram in the next iteration of this DG to depict the high-level design of your software and make it more detailed.
| v2.0 | foreigner | view my entries in a different currency | read my incomes and expenses in a currency I am familiar with | | ||
| v2.0 | user | set a target for my balance | improve my financial management | | ||
| v2.0 | user | see the target i have set | remind myself of my target | | ||
| v2.0 | user | reset my income/expense lists or both | have a fresh list | | ||
## Non-Functional Requirements |
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.
Please add these details in the next iteration in your DG
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.
Eg. Add sample test cases for commands
Overall, a detailed DG draft. Great job! Do note to correct the minor errors, add on to manual testing as well as provide a system architecture diagram. Hope to see more improvements in the next iteration of the DG. |
docs/DeveloperGuide.md
Outdated
### Target user profile | ||
|
||
{Describe the target user profile} | ||
Target users are people who are keen on improving their financial accountability |
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.
Could this be elaborated on further? For example, what exactly is financial accountability?
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.
To add on the above. Perhaps a brief description of how this product is able to help people to improve their financial accountability can be briefly stated?
docs/DeveloperGuide.md
Outdated
|
||
### Value proposition | ||
|
||
{Describe the value proposition: what problem does it solve?} | ||
The value proposition of ChChing is its ability to track income and expenses on a daily basis. |
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.
Could this be elaborated on further? How else does this application solve the problem better than other similar applications?
| Version | As a ... | I want to ... | So that I can ... | | ||
|---------|-----------|-----------------------------------------|-----------------------------------------------------------------------------------------| | ||
| v1.0 | new user | see usage instructions | refer to them when I forget how to use the application | | ||
| v1.0 | user | add new expense to the records | record all my expenses | | ||
| v1.0 | user | add new income to the records | record all my incomes | | ||
| v1.0 | user | view all the records | refer to them when I forgot my expenses and incomes | | ||
| v1.0 | user | edit the records | modify/fix if the records is changed/wrong | | ||
| v1.0 | user | know current balance | aware how much money do I have left | | ||
| v2.0 | user | edit my existing entries | rectify or update any entries without having to enter another entry and delete an entry | | ||
| v2.0 | user | find specific entries | refer to specific entries when necessary | | ||
| v2.0 | foreigner | view my entries in a different currency | read my incomes and expenses in a currency I am familiar with | | ||
| v2.0 | user | set a target for my balance | improve my financial management | | ||
| v2.0 | user | see the target i have set | remind myself of my target | | ||
| v2.0 | user | reset my income/expense lists or both | have a fresh list | |
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 user stories!! Your application accurately solves the problems listed.
docs/DeveloperGuide.md
Outdated
The ```printSelector()``` method will print all the available currencies in the selector hashmap. | ||
The selected currencies will be marked with a ```[X]``` and the unselected currencies will be marked with a ```[ ]```. | ||
|
||
![SetCurrencyCommand_sequence_diagram.png](images%2FSetCurrencyCommand_sequence_diagram.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.
To add on to the above, the formating of the sequence diagrams themselves are slightly inconsistent, in terms of the colour of the boxes of the instances, and the word font colour of the operations.
update my repo
Update Thomas's Project Portfolio Page
Update UG and DG
Fix Storage bug
Update runtest's expected output to allow Java CI to pass
Add Non-functional Requirements in Developer Guide
currency test
Update PPP
Remove console handler for logging
v2.1 wrap up
Update PPP
Update show target to 2dp
Thomas page breaks
Update UG
Add page break to neaten make Developer Guide pdf compatible
category find
Update PPP
ChChing is a desktop app for tracking spending, and it uses a Command Line Interface (CLI) for managing finances. If you are someone who needs a simple interface to get a better hold of your finances, this app is for you!