[CS2113-T14-2] BagPacker#61
Conversation
add authorship for Sherlock-YH
varnika1402
left a comment
varnika1402
left a comment
There was a problem hiding this comment.
Overall good explanation of some commands with UML diagram. It would be better to include more detail in the DG.
| When `contains()` returns `true`, method `addItem(item)` will be interrupted and a `DuplicateItemException` will be thrown from `packingList`, which will be caught by `a`. If `contains()` returns false, the item will be added onto `packingList`. | ||
|
|
||
| In both scenarios, `ui.printToUser` will be called to print a message to the user. In the former case, `ExistItemError` will be printed, while `AddSuccess` will be shown if the item was added with no issues. | ||
|
|
There was a problem hiding this comment.
good workflow explanation of the command
| | v2b | user | Save my packing list | keep track of my packing list even after leaving the app | | ||
|
|
||
| ## Non-Functional Requirements | ||
|
|
There was a problem hiding this comment.
should be better to add in instructions for manual testing to get an overview of all commands?
| original source as well} | ||
|
|
||
| ## Design & implementation | ||
|
|
There was a problem hiding this comment.
Should be better to mention the main components of the app architecture to understand the overview of components used?
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
good use of UML diagram to understand workflow of the command.
DavidVin357
left a comment
DavidVin357
left a comment
There was a problem hiding this comment.
The existing sequence diagrams are good enough, but the DG definitely lacks class diagrams.
Otherwise, the description gives enough information to understand the product and its implementation, though more diagrams would decrease mental effort needed.
|
|
||
| When using the `add` function, we have decided to implement a passive function that checks whether the item with the same name already exists in the packingList. | ||
|
|
||
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. |
There was a problem hiding this comment.
Maybe class diagram would better clarify interactions between the classes rather than text description?
There was a problem hiding this comment.
Nice decision to include the reference frame!
There was a problem hiding this comment.
The getDescription calls get pretty confusing here - maybe try to create another activation bar for detaching them to clarify the functionality?
|
|
||
| Delete command is used to delete an item from the packing list. | ||
|
|
||
| Mechanism: ```DeleteCommand.execute()``` calls the ```PackingList.deleteItem()``` method from the ```PackingList``` class which executes the ```ArrayList.remove()``` method to remove the item from the ```PackingList``` ArrayList. |
There was a problem hiding this comment.
Would be great to see the diagrams here!
There was a problem hiding this comment.
Does Exception thrown back statement mean actual throw or just showing the error to the user? Would be nice to have it more clearly stated or visualized!
| |--------|----------|---------------|------------------| | ||
| |v1.0|new user|see usage instructions|refer to them when I forget how to use the application| | ||
| |v2.0|user|find a to-do item by name|locate a to-do without having to go through the entire list| | ||
| | Version | As a ... | I want to ... | So that I can ... | |
There was a problem hiding this comment.
Good job with specifying the user stories by version! Gives a good understanding of the actual program and the progress made
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the | ||
| original source as well} | ||
|
|
||
| ## Design & implementation |
There was a problem hiding this comment.
One more idea would be to create a detailed class diagram of the Exception classes - this would greatly help developers with what to use when handling new exceptions for new functionality
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
Should contains(item) be referenced as an sd block rather than as the condition in the alt block?
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
Should there be an additional activation bar when packingList calls its add method?
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
Should the calls to ui be pointed at its lifeline rather than the entity, and be aligned to its activation bar? And, shouldn't the alt block contain the two possibilities of calls to the UI entity?
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  | ||
| When `execute()` is called in `a`(object of class `AddCommand`), the `addItem(item)` method is called in the object `packingList`. This method will see if method `contains()` will return `true`. |
There was a problem hiding this comment.
Would it be a good idea to give the AddCommand instance a more descriptive name?
| The `contains()` method, which is shown as a reference on the right of the UML diagram, is a boolean method that loops through all items in the packingList, and returns `true` if any of the existing items have the same description as the item to be added in `toAdd`, and `false` otherwise. | ||
|
|
||
| When `contains()` returns `true`, method `addItem(item)` will be interrupted and a `DuplicateItemException` will be thrown from `packingList`, which will be caught by `a`. If `contains()` returns false, the item will be added onto `packingList`. | ||
|
|
||
| In both scenarios, `ui.printToUser` will be called to print a message to the user. In the former case, `ExistItemError` will be printed, while `AddSuccess` will be shown if the item was added with no issues. | ||
|
|
There was a problem hiding this comment.
Would it be better to place this explanation in front of the diagram?
| #### Bye Command | ||
| ```ByeCommand``` is used to exit the BagPacker application. | ||
|
|
||
| Mechanism: ```ByeCommand.execute()``` updates the static boolean ```isBagPackerRunning``` to be false. |
There was a problem hiding this comment.
In which class is the isBagPackerRunning static variable stored? Would it be a good idea to specify?
|
|
||
| Delete command is used to delete an item from the packing list. | ||
|
|
||
| Mechanism: ```DeleteCommand.execute()``` calls the ```PackingList.deleteItem()``` method from the ```PackingList``` class which executes the ```ArrayList.remove()``` method to remove the item from the ```PackingList``` ArrayList. |
There was a problem hiding this comment.
Nice and concise explanation of mechanism
| 2. Create command object - The ```Parser``` class creates a corresponding command object of the relevant command | ||
| 3. Execute command object - ```runBagPacker()``` method executes the ```.execute()``` method (overridden by child classes) of the command object | ||
| which runs the actual command function | ||
|
|
There was a problem hiding this comment.
Would be nice to see some class and sequence diagrams
| | v2a | user | find an item by name | find the pack status of an item without having to go through the entire list | | ||
| | v2a | user | remove my packing list | clear my list once I am done packing | | ||
| | v2a | user | Specify the quantity of an item I need to pack | keep track of individual item quantities being packed | | ||
| | v2b | user | Save my packing list | keep track of my packing list even after leaving the app | |
There was a problem hiding this comment.
very well-managed by specifying the version to be implemented and prioritizing the key features.
| #### Help Command | ||
| Help command is used to exit the BagPacker application. | ||
|
|
||
| Execute: ```HelpCommand.execute()``` prints the following help message. |
There was a problem hiding this comment.
Would be nice to know what type of help commands are available.
Gan868611
left a comment
Gan868611
left a comment
There was a problem hiding this comment.
Should add detailed diagram to all the features
| @@ -1,29 +1,115 @@ | |||
| [DeveloperGuide.md](DeveloperGuide.md) | |||
| # Developer Guide | |||
|
|
|||
There was a problem hiding this comment.
Would be good to include table of contents in the top of the DG
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the | ||
| original source as well} | ||
|
|
||
| ## Design & implementation |
There was a problem hiding this comment.
Would be good to include Main architectural design and class diagram for each class to showcase the whole project design
| It then updates the ```quantity``` variable according to the quantity inputted by the user. | ||
|
|
||
|
|
||
| #### Preventing duplicate items |
There was a problem hiding this comment.
Preventing duplicate items and add command now are on the same layer which makes it ambiguous. Since preventing duplicate items is under add command then it should be one layer down
|
|
||
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. |
There was a problem hiding this comment.
I think specifying the below diagram as Sequence Diagram would be a good idea
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
- Exception handling in Sequence Diagram is ambiguous.
- Missing self invocation activation bar for
.add(item) - Suggest to check out the correct method to showcase static method.(It should point to activation bar)
- Incorrect use of Reference frame, should check out module website( should have ref and sd frame)
- Return of
trueandfalseshould be dashed line. - As it is already in loop frame, there is no need to do self-invocate the whole activation bar.
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. | ||
|  |
There was a problem hiding this comment.
Maybe there should be two Ui entities instead of one, as the two possible calls does not call the same Ui object.
|
|
||
| {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} | ||
| {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.
It may be better to give a brief overview of how your application works ] at the beginning of the docs, including the main commands and components, before diving into each component individually.
|
|
||
| This is done through the `contains()` method in class `PackingList()`, which is called during `execute` in an `AddCommand` object. | ||
|
|
||
| Below is the UML diagram showing what occurs during `add` function. |
There was a problem hiding this comment.
Maybe it is better to add the return of control in your diagram.
|
|
||
| {Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.} | ||
| ### Command Mechanisms: | ||
| For all valid commands, the mechanism of implementation are as follows: |
There was a problem hiding this comment.
It would be better if you could try standardising whether to add the full stop at the end of the sentence.
Add Mac prompts to UG
This reverts commit 961ae6c.
Edit code formatting and wording
Change error message
Create packall feature
Save and load feature added
fix - improve load() function in Storage class to read quantities of above 9
Update User Guide
Update PPP
Bug fixing
Add line breaks
Fix bug - characters get cut off in pdf
Refactor Linus' PPP
Add line break
Fix DG broken link and more
code block fix
handled test fix
fix io tests
10 participants
BagPacker is an app to help travelers manage their packing lists.