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

[CS2113-T15-4] StockMaster #45

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

Conversation

fxe025
Copy link

@fxe025 fxe025 commented Mar 7, 2024

StockMaster helps small business owners manage inventory of their business. It is optimized for CLI users so that frequent tasks can be done faster by typing in commands.

yeozongyao pushed a commit to yeozongyao/tp that referenced this pull request Mar 29, 2024
Copy link

@fungg0 fungg0 left a comment

Choose a reason for hiding this comment

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

I think your DG could use more descriptions to show how the program works and interact within.

Copy link

Choose a reason for hiding this comment

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

I think you can add more visuals to aid in explaining your design and implementation

Copy link

Choose a reason for hiding this comment

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

The document looks neat!

### Storage
Storage class contains methods to write description of items to the file `./StockMasterData.txt`,
and retrieve information from the file when program restarts.
### UI
Copy link

Choose a reason for hiding this comment

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

I think your UI and Exception class is missing a description

### Storage
Storage class contains methods to write description of items to the file `./StockMasterData.txt`,
and retrieve information from the file when program restarts.
### UI

Copy link

Choose a reason for hiding this comment

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

I think you can add some description regarding your implementation for the functionalities

@@ -7,31 +7,67 @@
## Design & implementation

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
### Exception
### Itemlist
Itemlist class is an object which contains items to be added to the stock inventory list. Able to add items, remove functions, edit items inside
Copy link

Choose a reason for hiding this comment

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

Seems like you meant to say remove items instead of remove functions.

Comment on lines 40 to 45
* has a need to manage a significant number of inventory products
* able to track revenue/loss of the business
* prefer desktop apps over other types
* can type fast
* prefers typing to mouse interactions
* is reasonably comfortable using CLI apps
Copy link

Choose a reason for hiding this comment

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

Target users are clearly defined. Good job!

Copy link

Choose a reason for hiding this comment

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

Do consider adding in more visuals (sequence diagrams, class diagrams, etc.) for explanatory purposes.

Copy link

@T0nyLin T0nyLin left a comment

Choose a reason for hiding this comment

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

Looks clean, but needs more examples and images to help with the DG.

* list: list all items in the inventory
* help: list all commands
* exit: exit StockMaster

Copy link

Choose a reason for hiding this comment

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

Hopefully, you can explain what the contents of each methods are and how the methods communicate with each related methods. This DG is to teach an interested developer how to contribute to the codebase.

Choose a reason for hiding this comment

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

The sequence diagram is clear and easy to understand, good job!

Choose a reason for hiding this comment

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

You should add the return arrows to your sequence diagram

Choose a reason for hiding this comment

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

Maybe you could add both the activation bar and return arrows

Choose a reason for hiding this comment

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

You could try adding the activation bar and return arrows as well

used in the product.

{Describe the design of the product. Use UML diagrams and short code snippets where applicable.}
### Exception

Choose a reason for hiding this comment

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

I think you could elaborate more on how your exception works

Choose a reason for hiding this comment

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

  1. missing closing bracket in print
  2. add return arrow when necessary :)

Copy link

@ZhuSijia0711 ZhuSijia0711 left a comment

Choose a reason for hiding this comment

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

maybe can include some class diagram and object diagram to show variance :)

Choose a reason for hiding this comment

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

maybe the diagram is a bit small

Choose a reason for hiding this comment

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

good to see self invocation here !

and retrieve information from the file when program restarts.
### UI

<<<<<<< HEAD

Choose a reason for hiding this comment

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

please delete the unnecessary lines :) !

Copy link

Choose a reason for hiding this comment

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

No problem for this diagram

Copy link

Choose a reason for hiding this comment

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

I am a bit confuse what does "BESTSELLER", "TOTAL_PROFIT", "TOTAL_REVENUE" means?

Copy link

Choose a reason for hiding this comment

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

Good job in use red to diftinquish the parts.

Copy link

Choose a reason for hiding this comment

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

Good and clear format!

Shu and others added 30 commits April 15, 2024 22:39
Update Storage Sequence Diagram
Change UG and DG wording
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.