Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CS2113T-F11-4] CLIvershelf #43

Open
wants to merge 817 commits into
base: master
Choose a base branch
from

Conversation

yuejunfeng0909
Copy link

CLIvershelf helps bookstore owners to better manage their inventory and finances. It is optimized for CLI users so that frequent tasks can be done faster by typing in commands.

Copy link

@Yuxinn-J Yuxinn-J left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, The documentation looks good to me, with enough visuals.

4. `Main` calls `Command` object to `execute()`, and returning a `resultString`
5. `Main` instantiates `UI` component to print the `resultString`

![](diagrams/GeneralProgramFlowSequenceDiagram.png)
Copy link

@Yuxinn-J Yuxinn-J Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

I like the usage of minimal notation of the sequence diagram, very clear and do not result in ambiguities.

5. `Command` then checks the `ExitCommand` on whether the program should exit.
6. In the absence of `ExitCommand`, UI then takes over to prompt and process the next user input.

![](diagrams/seedu_duke_logic.drawio.svg)
Copy link

@Yuxinn-J Yuxinn-J Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

The Attributes of Class Parser is too complicated. It is not necessary to contain every details of a class. Should the Atrributes compartment be omitted instead? 😄


### Model component

![](diagrams/seedu_duke_model.drawio.svg)
Copy link

@Yuxinn-J Yuxinn-J Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Is the position of the Multiplicity and Roles correct? Multiplicity and Roles should be close to the class, not in the middle of the connections.

Comment on lines 4 to 26
1. [Introduction](#introduction)
2. [Setting up](#setting-up)
3. [Acknowledgements](#acknowledgements)
4. [Design](#design)
1. [Architecture](#architecture)
2. [UI Component](#ui-component)
3. [Logic Component](#logic-component)
4. [Model Component](#model-component)
5. [Storage Component](#storage-component)
5. [Implementation](#implementation)
1. [Adding an item](#adding-an-item)
2. [Editing an item](#editing-an-item)
3. [Listing all items](#listing-all-items)
4. [Getting an item](#getting-an-item)
5. [Selling an item](#selling-an-item)
6. [Generating sales report](#generating-sales-report)
7. [Getting help](#getting-help)
8. [Exiting the program](#editing-an-item)
6. [Product Scope](#product-scope)
7. [User stories](#user-stories)
8. [Non-Functional Requirements](#non-functional-requirements)
9. [Glossary](#glossary)
10. [Instructions for manual testing](#instructions-for-manual-testing)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the Anchors using here! It is easy to for me to reference to each specific parts. 😃

Comment on lines 298 to 308
### Removing a shelf

### Adding an item

### Deleting an item

### Getting information of an item

### Listing the items

### Editing an item

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! Missing contents


### Editing an item

### Getting a Report

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the Getting a Report Part, It seems hard for me to read. Will it look more friendly if adjusting the format?

Comment on lines 313 to 314
2. Test case: `report t/stats`
Expected: Shows a report of the statistics (total cost, total income, total profit) of the sales from the beginning of time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

For example,

Suggested change
2. Test case: `report t/stats`
Expected: Shows a report of the statistics (total cost, total income, total profit) of the sales from the beginning of time.
2. Test case: `report t/stats`
*Expected:* Shows a report of the statistics (total cost, total income, total profit) of the sales from the beginning of time.

Copy link

@VishalJeyaram VishalJeyaram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great job! The diagrams are simple to comprehend. You just need ot finish up on the explanations as some are missing.

3. `Logic`: Parses and executes the user input commands.
4. `Model`: Holds the data of the App in memory
5. `Storage`: Reads data from, and writes data to, the hard disk.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you gave a brief description of each of your classes. This makes it easier to understand why your sequence/class diagrams implemented in such a manner.

Comment on lines 188 to 208
### Editing an item

#### Design considerations:

### Listing all items

#### Design considerations:

### Getting an item

#### Design considerations:

### Selling an item

#### Design considerations:

### Generating sales report

#### Design considerations:

### Getting help
Copy link

@VishalJeyaram VishalJeyaram Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that there's missing content here. Perhaps it may not be necessary to have design considerations for every single aspect here. It would be better to have an overall design considerations to encompass everything in this part.

3. Getting a report when none of the items are sold
1. Pre-requisite: `list shlv/soldItems` to check that there does **not** exist a shelf name sold items
2. Test case: `report t/stats`
Expected: Shows a message none of the items have been sold to generate the statistics report

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be good to show the message that is printed instead of saying that a message will be printed.

|v2.0|user|retrieve a list of items low in stock|replenish items low in stock|
|v2.0|user|mark an item as sold|remove item from inventory list and add to revenue|
|v2.0|user|add the total cost of all the items|know the total cost and deduct from revenue to find profit|
|v2.0|user|view the monthly sales report|know if I am making a profit|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that you structured your user stories in terms of v1.0 and v2.0 to give the reader an idea of how your app is progressing.

yuejunfeng0909 and others added 28 commits November 6, 2021 11:48
# Conflicts:
#	docs/DeveloperGuide.md
Fix links in DG manual testing
Refactored classes & packages
Minor updates to DG for storage component
Update UI diagrams, rename some diagrams
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants