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-F13-2] BrokeMan #51

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

Conversation

namsengi11
Copy link

BrokeMan helps students running on low budget by organizing their budget usage and expenditures. It provides various features to help students minimize their spending and raise awareness on their spendings. It is optimized for CLI users so that budget and expenditure recording can be done quicker through entering commands.

kyrixn pushed a commit to kyrixn/tp that referenced this pull request Mar 16, 2023

![Architecture Diagram](images/ArchitectureDiagram.png)

The ***architecture diagram*** given above is explains the high level design of the program.

Choose a reason for hiding this comment

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

There seems to be a typo in this sentence


---

|Version| As a ... | I want to ... | So that I can ... |

Choose a reason for hiding this comment

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

User stories are repeated in the DG, there are 2 tables


---

## implementation

Choose a reason for hiding this comment

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

"i" is in lowercase

@@ -1,38 +1,253 @@
# Developer Guide

Choose a reason for hiding this comment

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

Perhaps can add more diagrams of different types


## Design

### Architecture

Choose a reason for hiding this comment

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

Architecture diagram is clear and well explained

Copy link

@denzelcjy denzelcjy left a comment

Choose a reason for hiding this comment

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

Nice work on trying to segment the information for better readability, can consider using more bullets, whitespaces and different font sizes in order to keep it more consistent. Can consider adding sequence diagrams, and maybe a class diagram to show how different classes interact with each other. Overall good job! 👍

- [UI component](#ui-component)
- [Parser component](#parser-component)
- [Storage component](#storage-component)
- [Common classes](#common-classes)

Choose a reason for hiding this comment

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

The link here for the common classes does not seem to be working, perhaps edit the content in the link bracket to be #common-class for it to work?

Comment on lines 13 to 14
4. [Appendix: Requirements](#appendix--requirements)
5. [Appendix: Instructions for manual testing](#appendix--instructions-for-manual-testing)

Choose a reason for hiding this comment

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

These 2 links also do not seem to work when I clicked it, perhaps consider changing the '--' to '-' after 'appendix'?

Comment on lines 19 to 23
x
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}

## Design & implementation
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the
original source as well}

Choose a reason for hiding this comment

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

there seems to be duplicate sentences here, can consider updating and filling it up whenever possible!

Comment on lines 34 to 46
![Architecture Diagram](images/ArchitectureDiagram.png)

The ***architecture diagram*** given above is explains the high level design of the program.

Given below is a quick overview of the main components and how they interact with each other.

**Main components of architecture**

`BrokeMan` has one class [`Main`](https://github.com/AY2223S2-CS2113-F13-2/tp/blob/master/src/main/java/seedu/brokeMan/BrokeMan.java), which is responsible for:
- At program launch: Initialises the components in the correct sequence, and connect them up with each other
- At program termination: Shuts down the components and invokes cleanup methods where necessary.

[`Common`](#common-class) represents a collection of messages used by multiple other components.

Choose a reason for hiding this comment

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

image
A little confused with the architecture diagram, in line 46 it mentions that "common represents a collection of messages used by multiple other components, but there seems to be no arrow associated with the "common" component in the diagram, is it supposed to be intended this way?


---

## implementation

Choose a reason for hiding this comment

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

Can consider to change it to "Implementation" instead

Comment on lines 73 to 98
### Entry

Entry class is the underlying superclass for Expense and Income classes. It establishes the common attributes and
methods that are necessary to represent Expenses and Incomes. Abstract class is used to represent their common features
to maximize code reusability and increase maintainability.

Private attributes

Info: String that stores the description of the entry

Amount: Double that stores the monetary value of entry

Time: LocalDateTime that stores the date and time of entry

Category: Category that stores the type tag of entry

**Methods**

Getters can be used to provide the private attributes to other classes

editDescription(), editAmount(), editTime(), editCategory()

* Takes in corresponding parameters to edit the private attributes.
* Used by EntryList to make edits

isSameMonth()

Choose a reason for hiding this comment

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

Nice work having a short summary as a description, can consider bolding "Private attributes" to make it aligned in formatting with the "Methods" that you already bolded below.
Can also consider adding bullets to indent the subpoints "Info, Amount, Time, Category" to increase readability!
Alternatively, maybe you could use a table as another way to display these details :)

Comment on lines 226 to 234
| Priority | Version | As a ... | I want to ... | So that I can ... |
|----------|---------|----------|-------------------------------------------|--------------------------------------------------|
| `* * *` | v1.0 | user | add, delete, edit, and list my income | manage my income |
| `* * *` | v1.0 | user | add, delete, edit, and list my expenses | manage my expenses |
| `* * *` | v1.0 | user | set and view my budget | set expectation of how much money I should use |
| `* * *` | v1.0 | user | view how much of the budget I spend | manage and change my spending habit as necessary |
| `* * *` | v1.0 | user | view all command that I can enter | get help on the features if necessary |
| `* * *` | v2.0 | user | list monthly expenses, income, and budget | refer to financial status in previous months |
| `* * *` | v2.0 | user | save all my income and expenses entered | so that I can refer to it next time I return |

Choose a reason for hiding this comment

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

This is a second User Stories table that is almost similar to the first one above, is this supposed to be combined or meant to be an additional table? Can consider providing just 1 table to reduce confusion 👍

### SaveExpense, SaveIncome

The SaveExpense and SaveIncome class deal with saving in the user inputted data locally so that it can be later accessed.
**Methods**

Choose a reason for hiding this comment

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

image
Can consider adding a line break here so that "Methods" is on a new line

Comment on lines 142 to 146
SortEntryByAmount()

* Returns a sorted list of entry by amount of entry.

SortEntryByDate()

Choose a reason for hiding this comment

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

are these 2 methods meant to be written as sortEntryByAmount and sortEntryByDate() instead? To follow the camelCase convention for method naming

Copy link

@Mnsd05 Mnsd05 left a comment

Choose a reason for hiding this comment

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

Overall, your developer guide is easy to understand, well done!

Comment on lines 31 to 34

![Architecture Diagram](images/ArchitectureDiagram.png)

The ***architecture diagram*** given above is explains the high level design of the program.
Copy link

Choose a reason for hiding this comment

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

You may add the Main class to the diagram

Comment on lines 53 to 63
### Ui component

### Parser component

### Command component

### Storage component

### EntryList component

### Common class
Copy link

Choose a reason for hiding this comment

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

This part is empty, you might want to delete or add the sequence diagrams for each component

Comment on lines 36 to 251
## Instructions for manual testing
## Appendix: Instructions for manual testing

{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing}
Copy link

Choose a reason for hiding this comment

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

You can try to include how you give some input to the program and how the program should respond. For example:

Input: abc

Expected: Oops, I don't know what that means

Comment on lines 224 to 232
| Priority | Version | As a ... | I want to ... | So that I can ... |
|----------|---------|----------|-------------------------------------------|--------------------------------------------------|
| `* * *` | v1.0 | user | add, delete, edit, and list my income | manage my income |
| `* * *` | v1.0 | user | add, delete, edit, and list my expenses | manage my expenses |
| `* * *` | v1.0 | user | set and view my budget | set expectation of how much money I should use |
| `* * *` | v1.0 | user | view how much of the budget I spend | manage and change my spending habit as necessary |
| `* * *` | v1.0 | user | view all command that I can enter | get help on the features if necessary |
| `* * *` | v2.0 | user | list monthly expenses, income, and budget | refer to financial status in previous months |
| `* * *` | v2.0 | user | save all my income and expenses entered | so that I can refer to it next time I return |
Copy link

Choose a reason for hiding this comment

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

Well done! I find that priority is a good feature of your user stories as it gives an idea of the most important features in your product

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.

8 participants