Added Quote Command #257
Added Quote Command #257
Conversation
There was a problem hiding this comment.
With this current design, QuoteCommand has a dependency on a Ui. It is not necessary for a Command to know of the existence of a Ui in the first place. Likewise, it is not the job of the Ui to handle quotes - its only role is to process input and write output, wherever this output may originate from.
Referring to our architectural design, these classes are in 2 different components, and are already decoupled.
In this case, I would suggest having a Quotes class which effectively stores only String quotes (either as constants or in an array). Then, implement a static getter method within this class that enables the retrieval of a randomized quote.
Thus, when calling execute() of the QuoteCommand, a random quote will be retrieved from this Quotes class through the getter method. Then, we return this quote back to BinBash, and print it using Ui.talk().
Likewise, for the randomized startup quote, we can allow BinBash to make a call to the Quotes class directly through the aforementioned getter method.
For this Quotes class, I would suggest putting it in the ui package. As you've reasonably stated, the quotes are indeed associated with the UI of the application, and this should preserve the logical structure of the code-base.
Additionally, I would suggest that Quotes remain as a utility class which need not be explicitly instantiated using Quotes quotes = new Quotes(). As mentioned earlier, we can access the quotes in this class using a static getter method. In turn, the quotes should be declared static as well (or, store them in a static String array).
|
I echo Kota's previous comments as well - I am open to implementing this feature (in spite of it being a non-functional requirement). However, I would kindly request that necessary changes are made to the design of this feature, before we proceed with the implementation of this feature into the application. |
docs/UserGuide.md
Outdated
| ### Getting Inspirational Quotes: `quote` | ||
|
|
||
| > This command fetches a random quote to uplift your spirits. | ||
|
|
||
| Need a little motivation? The quote command retrieves random inspirational messages to brighten your day. Whether you're feeling stuck or just need a boost, BinBash has got you covered! | ||
|
|
||
| Format: `quote` | ||
|
|
||
| > ℹ️ BinBash brings you a variety of inspiring messages to keep you motivated throughout your inventory management journey. So go ahead, type quote and let the positivity flow! | ||
| * [Back to table of contents](#table-of-contents) | ||
| --- | ||
|
|
There was a problem hiding this comment.
Consider adding an example of the quote output.
Nonetheless, good description. There's a strong display of user-friendliness.
docs/UserGuide.md
Outdated
| | **restock** | `restock -n ITEM_NAME -q ITEM_QUANTITY` <br> `restock -i ITEM_INDEX -q ITEM_QUANTITY` | Increases the quantity of an item after restocking. | | ||
| | **update** | `update -n ITEM_NAME -d ITEM_DESCRIPTION -q ITEM_QUANTITY -e EXPIRY_DATE -s SALE_PRICE -c COST_PRICE -t THRESHOLD` <br> `update -i ITEM_INDEX -d ITEM_DESCRIPTION -q ITEM_QUANTITY -e EXPIRY_DATE -s SALE_PRICE -c COST_PRICE -t THRESHOLD` | Updates the details of an existing item in the inventory. | | ||
| | **profit** | `profit` | Displays the total profit earned from the inventory. | | ||
| | **quote** | `quote` | Displays a random quote on the screen. | |
There was a problem hiding this comment.
Thanks for updating the 'Command Summary' table. 👍
This is extremely minor, but we would need to update the Key Definitions > Placeholders > COMMAND_WORD section as well.
| Assertions.assertEquals(thrown.getMessage(), "Please provide a positive number."); | ||
| } | ||
|
|
||
| // New test case: Parsing with missing item ID | ||
| @Test |
There was a problem hiding this comment.
Thanks for adding more test to boost test coverage!
| @Test | ||
| void execute_removeDuplicateNamesOnlyOneRemoved() { | ||
| itemList.addItem("retail", "test", "Another test item", 1, | ||
| LocalDate.now(), 5.00, 2.00, 4); // Adding a duplicate item | ||
|
|
||
| assertEquals(1, itemList.getItemCount()); | ||
|
|
||
| DeleteCommand deleteCommand = new DeleteCommand("test"); | ||
| deleteCommand.execute(itemList); | ||
|
|
||
| assertEquals(0, itemList.getItemCount()); | ||
| assertTrue(deleteCommand.hasToSave()); | ||
| } |
There was a problem hiding this comment.
Perhaps this can be removed/changed to something else, to reflect our current app behaviour.
Adding of duplicate items is not allowed, and that test has already been done in AddCommandTest.
| @@ -56,4 +55,18 @@ void execute_invalidItemName_itemNotRemovedFromItemList() { | |||
|
|
|||
| assertEquals(1, itemList.getItemCount()); | |||
| } | |||
|
|
|||
| @Test | |||
| void execute_removeDuplicateNamesOnlyOneRemoved() { | |||
There was a problem hiding this comment.
Consider rephrasing the method name to follow the the JUnit naming convention.
(I think there is a missing _ between 'Names' and 'Only')
No description provided.