Skip to content
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-F14-1] Brokeculator #66

Open
wants to merge 541 commits into
base: master
Choose a base branch
from

Conversation

STeng618
Copy link

Brokeculator helps university students manage their expenses

Copy link

@wenenhoe wenenhoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally good job on the DG. I like the inclusion of a table of contents for ease of navigation. Left some comments for areas for improvement, do take a look at them

### Architecture

The UML diagram below shows the main relationships between the classes in the Brokeculator application.
![img.png](images/architecture.png)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the right symbols to annotate the accessors of the attributes and methods of the class in the diagram.


The UML diagram below shows the main relationships between the classes in the category feature.
![category_class.png](images/category_class.png)
</br>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of line break can be done with \ or <br>. This is currently being rendered in the html version of the DG.

![img.png](images/architecture.png)

### Category feature
**Implementation** </br>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The </br> tag is not rendered properly. It shows as text in the webpage. Perhaps consider puttting this tag on the next line?
image

Comment on lines 40 to 46
- `addCategory(String category)` - Adds a category to the set of categories
- `getCategoryListString()` - Returns a string representation of the set of categories
- `removeCategory(String category)` - Deletes a category from the set of categories

The `Category` class is supplemented by the following classes to interact with the user:
- `CategoryCommand` - This class is responsible for handling and executing the commands related to categories
- `CategoryParser` It is responsible for parsing the user input

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting inconsistency, there is no dash when describing CategoryParser.

image

2. The `Logic` class directs the user input to the `GeneralInputParser` class, which sees the `summarise` keyword
in the user input and directs it to the `SummariseParser` class
3. The `SummariseParser` class parses the user input and returns a `SummariseCommand` object or an `InvalidCommand` object
depending on whether the user input is valid or not. A `SummariseCommand` object would store relevant information from

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applies to all
tags below as well.

Comment on lines 151 to 154
| v1.0 | student | see a basic summary of my expenses to see how much i have spent in total | ------------------ |
| v1.0 | student | view the expenses I have logged | know how much I have spent |
| v1.0 | paranoid user | save my expenses into a file | backup locally via a file to prevent data loss |
| v1.0 | student | have the ability to add expenses | ------- |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could make the user stories more complete by filling in the 'so that I can' part.

@@ -1,29 +1,165 @@
# Developer Guide

* [Acknowledgements](#acknowledgements)
* [Setting up, getting started] (#setting-up-getting-started)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken hyperlink here.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants