[CS2113-T14-3] MyLedger#31
Conversation
…teAccount Branch created a createAccount method Ps will fixed ioredirection as program grows
There was a problem hiding this comment.
Alignment of the saveList() and saveExpenditureList to the start of the activation bar can be improved.
There was a problem hiding this comment.
Control of the diagram becomes ambiguous after addExpenditure(). The activation bars are mostly too long and not properly aligned.
There was a problem hiding this comment.
When the method is returned, the line should be dotted instead of solid. Alignment of the activation bars can be further improved.
thomasjlalba
left a comment
thomasjlalba
left a comment
There was a problem hiding this comment.
I think the Developer Guide is quite in depth and seems to explain quite well. Just some minor issues here and there in terms of the diagrams. I think if it is possible can reformat the pictures as there are some pictures that are very long and so its very small on the Github page. 🥇
| The following shows the UML diagram used for the parser component implemented in MyLedger. | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/parserOverview.png"> |
There was a problem hiding this comment.
At the bottom of the picture, there would need to be a closure in the activation bar of :MainInputParser, it seems like currently there is no closure
| The following shows the UML diagram used for the parser component implemented in MyLedger. | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/parserOverview.png"> |
There was a problem hiding this comment.
The picture in the Github Page also seems to be cut out (on the right side).
| updated with all the current expenditures in the expenditure array list. | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/saveList.png"> |
There was a problem hiding this comment.
I don't think the method call does not need to be underlined
| The sequence diagram below shows the interactions of a successful execution of the EditCommand | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/parserEdit.png"> |
There was a problem hiding this comment.
The termination (X) should be after the activation bar (by a dotted line), I don't think it should be terminated right at the end of activation bar.
yixuann02
left a comment
yixuann02
left a comment
There was a problem hiding this comment.
Overall, very in-depth explanation of diagrams, good job! But do take note of the multiple notation errors in your diagrams. Well done.
| <p align="center"> | ||
| <img src="team/images/UMLClassDiagramExpenditure.png" width="80%"> | ||
| <br/> | ||
| <i>Figure 3: UML diagram for the Expenditure Categories component</i> | ||
| </p> | ||
|
|
There was a problem hiding this comment.
Very clear and easy to read UML Diagram provided. Also, good explanation of the diagram.
| Below shows the UML diagram representing the `command` package. | ||
| <p align="center"> | ||
| <img src="team/images/umlCommandClassDiagram.png" width="100%"> | ||
| <br/> | ||
| <i>Figure 4: UML diagram for the command package</i> | ||
| </p> | ||
|
|
There was a problem hiding this comment.
Perhaps you could split the UML diagram into different parts. The UML diagram is too small and unreadable.
| <p align="center"> | ||
| <img src="team/images/saveList.png"> | ||
| <br/> | ||
| <i>Figure 5: Sequence diagram for TxtFileStatus</i> | ||
| </p> |
There was a problem hiding this comment.
The activation bar of a method cannot start before the method call arrives and a method cannot remain active after the method has returned. Do take note of this notation error.
| <p align="center"> | ||
| <img src="team/images/initializeList.png"> | ||
| <br/> | ||
| <i>Figure 6: Sequence diagram for the process of saveExpenditureList</i> | ||
| </p> | ||
|
|
There was a problem hiding this comment.
Similarly, the return for the ExpenditureList, Expenditure etc. should not be before the end of activation bar, and also do take note of the notation errors mentioned above.
jaredoong
left a comment
jaredoong
left a comment
There was a problem hiding this comment.
Overall clear developer guide with detailed and clear diagrams, just need to fix minor formatting error. Good job!
| The following shows the UML diagram used for the parser component implemented in MyLedger. | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/parserOverview.png"> |
There was a problem hiding this comment.
Image is cut off on the right side. Additionally, the figure is a little small due to the multiple options shown. One possible workaround could be just to include only 2 options just to show an example, while clearly stating that the other options are omitted from the diagram to prevent cluttering it.
|
|
||
| Below shows the UML diagram representing the `command` package. | ||
| <p align="center"> | ||
| <img src="team/images/umlCommandClassDiagram.png" width="100%"> |
There was a problem hiding this comment.
Good that .png format was used which ensured that the image remained clear even when zooming in to see the details. However, the details are quite small, so you might want to consider splitting it into different diagrams
| the mark, unmark and edit commands. This is because they have a similar sequence diagram as the functions parseAdd and | ||
| parseLendBorrow. The only difference is the condition, with the loop happening one, one and four time(s) respectively. |
There was a problem hiding this comment.
The formatting for parseAdd and parseLendBorrow could be changed to standardize with the top portion.
| `Expenditure`: | ||
| - Fields: `date`, `amount`, `description` | ||
|
|
||
| `AcademicExpenditure`**`: |
| The sequence diagram below shows the interactions of a successful execution of the EditCommand | ||
|
|
||
| <p align="center"> | ||
| <img src="team/images/parserEdit.png"> |
There was a problem hiding this comment.
Activation bar for getExpenditure(index) should start at the head of arrow instead of above it. The 'X' that represents termination could also possibly be shifted down to below the activation bar.
| - Navigate to the folder in command terminal and run the command `java -jar [filename].jar` | ||
| - Alternatively, double click on the JAR file to run the app. | ||
| #### Adding a record | ||
| 1. Adding an expenditure |
There was a problem hiding this comment.
Not sure why but the bullet point for this is not showing up correctly on the github pages, but you could possibly include a new line before this, which could fix the error.
|
|
||
|
|
||
| - Test case : `viewtype swimming` | ||
| - Expected : Invalid message will be shown with the respective error message, in this case an |
There was a problem hiding this comment.
For each of the error message, it would be good to add an example to give the user a clearer idea on what the error message would look like
Bug fixes & Improve code quality & Add PPP
- Fix bug where next repeat date resets when reading from save file - Add repeat date field to constructors of tuition and accommodation expenditures - Add new repeat date field for save file - Refactor code that processes the repeat date for tuition and accommodation expenditures
Enhance features
Reformat DG
Add page breaks for PPP
Finalized UG
Improve format of PPP
Remove numbering from PPP
new update formating
Final finalized DG
<br> Should not have errors anymore
more formatting DG
Fix style in PPP and create JAR
Reformat PPP
8 participants
This product is for university students who prefer CLI over GUI to manage their finances while in University, as this is when student income is minimal.