[CS2113-T15-1] LongAh!#44
Conversation
…ormatting Change successful removal message
yuki-zmstr
left a comment
yuki-zmstr
left a comment
There was a problem hiding this comment.
Hi! I think your DG is nice and detailed. You could accompany the text description with more diagrams, as people like to see diagrams more than read text! (As far as I understand:) )
| ```plantuml | ||
| @startuml | ||
| left to right direction | ||
|
|
||
| abstract class longah.commands.Command { | ||
| + {abstract} execute(Group) | ||
| } | ||
|
|
||
| class longah.node.Transaction { |
There was a problem hiding this comment.
You could try and save the .puml as an image, and embed that image into the .md file.
| Command <|-- AddCommand | ||
| Command <|-- AddMemberCommand | ||
| Command <|-- AddTransactionCommand | ||
| Command <|-- AddGroupCommand | ||
| Command <|-- DeleteCommand | ||
| Command <|-- DeleteMemberCommand | ||
| Command <|-- DeleteTransactionCommand | ||
| Command <|-- DeleteGroupCommand |
There was a problem hiding this comment.
I think you do not need to mention every Command, perhaps mention a few examples. That is enough to give the reader an overview of the inheritance. Currently the diagram looks rather long.
| Command <|-- AddCommand | ||
| Command <|-- AddMemberCommand | ||
| Command <|-- AddTransactionCommand |
There was a problem hiding this comment.
You can try to use the hide circle command to hide the 'circle C', as our textbook does not mention that we need to have those icons
| ### Chart | ||
|
|
||
| <ins>Overview</ins> | ||
|
|
||
| The Chart class is responsible for generating a visual representation of the transaction solution in the form of a chart. | ||
|
|
||
| The chart is displayed in a separate window and shows the individual balances among the group members. | ||
|
|
There was a problem hiding this comment.
Perhaps you could include a sample image of how the Chart will look like!
| <ins>Transaction Methods</ins> | ||
|
|
||
| - *parseTransaction*: Parses the user input to extract lender and borrowers, then adds them to the transaction. | ||
|
|
||
| - *addBorrower*: Adds a borrower to the transaction. | ||
|
|
||
| - *getLender*: Returns the lender of the transaction. | ||
|
|
||
| - *isLender*: Checks if a given member is the lender of the transaction. | ||
|
|
||
| - *isborrower*: Checks if a given member is a borrower in the transaction. | ||
|
|
||
| - *isInvolved*: Checks if a given member is involved in the transaction. | ||
|
|
||
| - *toStorageString*: Converts the transaction to a string format for storage. |
There was a problem hiding this comment.
I think in contrast to code that we write, documents could use more indentations, so that it's easy to spot headers. For example, you could indent the lines with tags.
| * *storageFolderPath*: A string containing the path to the storage directory specific to the group. | ||
| * *storageMembersFilePath*: A string containing the path to the `members.txt` directory associated with the group. | ||
| * *storageTransactionsFilePath*: A string containing the path to the `transactions.txt` directory associated with the group. | ||
| * *membersFile*: A File object representing the `members.txt` file associated with the group. | ||
| * *transactionsFile*: A File object representing the `transactions.txt` file associated with the group. | ||
| * *members*: A MemberList object representing the list of Members in the group. | ||
| * *transactions*: A TransactionList object representing the list of Transactions in the group. | ||
| * *scanners*: A size 2 array of Scanners to be used for loading data from the data storage files. The first Scanner in the array is used for reading from `members.txt` while the second is used for reading from `transactions.txt`. |
There was a problem hiding this comment.
Perhaps you could accompany this with a class diagram, showing the attributes!
claribelho
left a comment
claribelho
left a comment
There was a problem hiding this comment.
Diagrams are good, but there's a lack of class and object diagrams!
| Each `StorageHandler` instance creates `members.txt` and `transactions.txt` in their respective folders. The file format is as follows. | ||
|
|
||
| * `members.txt` - NAME | BALANCE | ||
| * `transactions.txt` - LENDER NAME | TRANSACTION TIME(if applicable) | BORROWER1 NAME | AMOUNT1 | BORROWER2 NAME... |
There was a problem hiding this comment.
Unclear on what this is representing
|
|
||
| <ins>Usage Example</ins> | ||
|
|
||
|  |
There was a problem hiding this comment.
Class names in sequence diagrams should have columns, eg :User
|
|
||
| <ins> Usage Example </ins> | ||
|
|
||
|  |
There was a problem hiding this comment.
Class LongAh calls new PINHandler(). This should be the label for the arrow from LongAh instead of Initialise. Also, as in this case the constructor of PINHandler() is called, the activation bar should be directly below the class name :PINHandler
| <ins>Usage Example</ins> | ||
|
|
||
| Adding a new transaction: | ||
|  |
There was a problem hiding this comment.
Class names should not be at the bottom of the sequence diagram
kyuichyi
left a comment
kyuichyi
left a comment
There was a problem hiding this comment.
Overall clean and neat DG, but diagrams have some slight issues.
|
|
||
| The UML diagram below provides an overview of the classes and their interactions within the LongAh application. | ||
|
|
||
|  |
There was a problem hiding this comment.
Diagram is too complicated. Do not need to include getters and setters in the digram.
|
|
||
| <ins>Methods</ins> | ||
|
|
||
| - *viewBalancesBarChart*(List<String> members, List<Double> balances): Generates a bar chart displaying member balances. It |
There was a problem hiding this comment.
Most methods shown do not include the variables (if have). Perhaps can standardize if including the variables or not.
|
|
||
| <ins>Implementation Details</ins> | ||
|
|
||
|  |
There was a problem hiding this comment.
Is there any way to simplify this diagram?
| <ins>Usage Example</ins> | ||
|
|
||
| Adding a new transaction: | ||
|  |
There was a problem hiding this comment.
Would be better if the lines did not intersect with the arrows.
|
|
||
| *Data Storage:* | ||
|
|
||
| Each `StorageHandler` instance creates `members.txt` and `transactions.txt` in their respective folders. The file format is as follows. |
There was a problem hiding this comment.
Suggest adding the path directory for where these text files are being saved in.
| The Transaction class is responsible for representing a single transaction in the LongAh application between 2 members. | ||
| It contains information about the lender, borrowers, and the amount involved in the transaction. | ||
|
|
||
|
|
| * Update upon change, not upon exit - This allows for data to be saved even if the application exits ungracefully. | ||
| * *checkTransactions* - Methods are provided to have a quick check to ensure that data from data storage is not corrupted. | ||
|
|
||
| ### Member and MemberList |
There was a problem hiding this comment.
Good use of a sequence diagram, though consider adding a class diagram as well since the class functionality is rather complicated
…sks shown in finding transaction and filter date methods now match with the full list.
…se decisions for the handling of time filter inputs of different types.
Update haowern98 PPP
PPP spacing updates
Format DG
DG Pagination
DG pagination
DG pagination
DG pagination
DG Pagination
DG Pagination
DG pagination
Docs pagination
Remove known issues
9 participants
LongAh! is a CLI-based application designed to help users track debts within friend groups and determine the least transaction method of settling these debts. It is optimized for busy people with large transaction quantities among friends.