[CS2113-W14-2] PlanPal #27
Conversation
| * @return true if continue to handle category commands. Otherwise, false. | ||
| */ | ||
| public boolean handleCategory(String description) { | ||
| try { |
There was a problem hiding this comment.
Perhaps shorten this method using guard clause instead.
| * @param description The description of the command | ||
| * @return true if continue to handle category commands. Otherwise, false. | ||
| */ | ||
| public boolean handleCategory(String description) { |
There was a problem hiding this comment.
Would prefer to use exception handling / assertions / guard clauses to deal with checking the functionality is successful, instead of using boolean return type.
| * | ||
| * @param description The category to be found | ||
| */ | ||
| public void searchCategory(String description) { |
There was a problem hiding this comment.
The naming of the method may mislead people that it will return some information.
| * @param list The list of items to search in. This should not be null. | ||
| * @param query The query string containing one or more keywords to search for. This should not be null. | ||
| */ | ||
| default <T> void findInList(ArrayList<T> list, String query) throws PlanPalExceptions { |
There was a problem hiding this comment.
Similar to previous issue; method name may mislead readers to expect something to be returned.
| description = inputParts[1].trim(); | ||
| contactManager.addContact(description); | ||
| return true; | ||
| // fallthrough |
There was a problem hiding this comment.
Inconsistent usage of comment.
The purpose of fallthrough is to ensure coders that they know code from previous switch cases are continued in the current case.
Since the return keyword is used, there is no need for fallthrough comments.
| fileManager.loadList(contactManager, "contacts.txt"); | ||
|
|
||
| while (isProcessing) { | ||
|
|
| break; | ||
|
|
||
| case EXPENSE_MANAGER: | ||
| // do something |
docs/Images/EditContact.drawio.png
Outdated
There was a problem hiding this comment.
Should the self call arrow for the editList() method of :ContactManager be pointing to the top of the activation bar instead
docs/DeveloperGuide.md
Outdated
|
|
||
| The **Architecture Diagram** given above explains the high-level design of the program. Given below is a quick overview of the main components. | ||
|
|
||
| ### Main Components |
There was a problem hiding this comment.
You can consider doing other UML diagrams for these components either Class / Object diagrams
docs/Images/AddContact.drawio.png
Outdated
There was a problem hiding this comment.
This diagram looks good just that the resolution is not to great. You could also consider abstracting the opt block with a ref
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
It would be good to add an explanation for the Edit and Set Category command like you guys did in the List Command
docs/DeveloperGuide.md
Outdated
| Welcome to the PlanPal Developer Guide! Thank you for taking an interest in the behind-the-scenes work of our product. We hope this document proves informative and useful for your work! | ||
|
|
||
| ## Table of Contents | ||
| {to be inserted at a later date} |
There was a problem hiding this comment.
Good that you are inserting a table of contents, hope that you get it up soon!
docs/DeveloperGuide.md
Outdated
| ## Acknowledgements | ||
|
|
||
|
|
||
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} |
There was a problem hiding this comment.
I understand that this is for planning purposes, but this can be removed once information has been added to the DG
docs/DeveloperGuide.md
Outdated
| - `UI`: In charge of printing messages | ||
| - `Logic`: Determines the command to execute | ||
| - `Storage`: Read and write data from hard disk | ||
| - `Command`: Specific commands for execution |
There was a problem hiding this comment.
Agree with the previous commenter that diagrams would be good to explain the components
docs/Images/AddContact.drawio.png
Outdated
There was a problem hiding this comment.
Instead of writing return as a return object, it would be good to be more specific as to what is actually being returned, i.e. return object
docs/Images/AddContact.drawio.png
Outdated
There was a problem hiding this comment.
The use of color to indicate which components the entities are from would also be helpful for sequence diagrams in this DG.
docs/Images/Architecture.drawio.png
Outdated
There was a problem hiding this comment.
Nice use of colors, forming a colorful diagram. As other sequence diagrams are with white background, it would be good to have a consistent background color.
docs/Images/ViewContact.drawio.png
Outdated
There was a problem hiding this comment.
Same comment as previous, on being more specific on what is being returned from each activation bar.
docs/Images/SetCategory.drawio.png
Outdated
There was a problem hiding this comment.
Same comment as previous, on being more specific on what is being returned from each activation bar.
docs/DeveloperGuide.md
Outdated
| |v1.0|NUS international student| delete a contact | remove a contact if I no longer need it | | ||
| |v1.0|NUS international student| edit a contact | amend any mistakes when creating the contact or if the number has been changed | | ||
| |v1.0|NUS international student| save my contacts | my contacts are still there when I exit and enter the app again | | | ||
| |v2.0|| | | |
There was a problem hiding this comment.
Should there be a 6th user story? I see that you have the version component filled but there rest of the user story (user type, action and purpose) is empty.
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
Should your alt diagram have a else case? Your alternative path has missing "else" case.
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
Does your alt diagram follow the format outlined in the professor’s notes? Does it include the operation arrows and activation bars for the if-else components?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Edit Command | ||
| The sequence diagram below illustrates the process for resolving the "edit" command. | ||
|  |
There was a problem hiding this comment.
There is inconsistent diagram backgrounds: this diagram has a black background, while the others use white.
There was a problem hiding this comment.
To add on to the inconsistency, should there be a standardisation for all the diagrams? In this case, should the object be only on the top, or should it be both on the top and bottom?
This case:
Most others:
|
|
||
| ### List Command | ||
| The sequence diagram below illustrates the process for resolving the "list" command. | ||
|
|
There was a problem hiding this comment.
Your self-invocation arrows in List Command feature sequence diagram do not align with the professor's notation. The bottom part of the arrow should be slanted.
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Edit Command | ||
| The sequence diagram below illustrates the process for resolving the "edit" command. | ||
|  |
There was a problem hiding this comment.
Your self-invocation arrows in Edit Command feature sequence diagram do not align with the professor's notation. The bottom part of the arrow should be slanted.
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
Your self-invocation arrows in Set Category Command feature sequence diagram do not align with the professor's notation. The bottom part of the arrow should be slanted.
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Edit Command | ||
| The sequence diagram below illustrates the process for resolving the "edit" command. | ||
|  |
There was a problem hiding this comment.
This section has missing explanation on how the Edit Command works. Would adding an explanation section be useful or is the sequence diagram enough?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
This section has missing explanation on how the Set Category Command works. Would adding an explanation section be useful or is the sequence diagram enough?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Add Command | ||
| The sequence diagram below illustrates the process for resolving the "add" command. | ||
|  |
There was a problem hiding this comment.
Should there be more context for the return? Should it be blank if it is returning void?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Edit Command | ||
| The sequence diagram below illustrates the process for resolving the "edit" command. | ||
|  |
There was a problem hiding this comment.
To add on to the inconsistency, should there be a standardisation for all the diagrams? In this case, should the object be only on the top, or should it be both on the top and bottom?
This case:
Most others:
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
Is there a need to put return ? Should just putting inCategory , true or false be enough to reduce clutter?
docs/DeveloperGuide.md
Outdated
| - `Storage`: Read and write data from hard disk | ||
| - `Command`: Specific commands for execution | ||
|
|
||
| ### Program Flow |
There was a problem hiding this comment.
Should there be state diagrams to better represent the flow? Should flow be more of 1. 2. 3. rather than point form bullet points?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Set Category Command | ||
| The sequence diagram below illustrates the process for resolving the "category" command. | ||
|  |
There was a problem hiding this comment.
Should there be visualization of user just like the add command?
Set Category:
Add:
docs/DeveloperGuide.md
Outdated
|
|
||
| Currently, only the contact features have been illustrated. |
There was a problem hiding this comment.
Should there be a diagram to explain or show the interactions or differences between Contact Manager and Expense Manager?
docs/DeveloperGuide.md
Outdated
|
|
||
| ### Add Command | ||
| The sequence diagram below illustrates the process for resolving the "add" command. | ||
|  |
There was a problem hiding this comment.
Should the font size of the words in the sequence diagram be consistent with the DG as mentioned in the lecture notes?
| ## Mode: Contact Manager | ||
| The class diagram below represents the contact book system | ||
|
|
||
| <u>Class Diagram for Contact Manager</u> |
There was a problem hiding this comment.
Is there a reason why different class diagrams look like they are generated from different software?
Also, do have the association labels, roles, and multiplicities.
Otherwise, actually a pretty good class diagram that shows the overview of the ContactManager!
Added test for search category in contactParser
Implemented saving and loading of contact category
Recurring expense
recurring is debugged to work for current month if current month is new
fix bugs and complete PPP
Complete PPP
added some more test
Complete PPP
update UG to provide more information about loading category data and…
updated portfolio link
fix UG format
handle picture bug in UG
test cases added for restore and backup functions
Edit DG diagrams
Add PPP link and made other formatting changes
DG editted
International students may struggle with coping to their new environment. This CLI-based personal assistant tool helps them to integrate to their new environment by giving them a way to quickly access and retrieve vital information.