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-T09-3] SITUS #25

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

Conversation

rainish2000
Copy link

Smart Inventory Tracking and Updating System (SITUS) will help restaurant inventory managers to keep track of and update their stock.


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)

Choose a reason for hiding this comment

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

image

Should the activation bar end when 'both lists of ingredients' is returned

Choose a reason for hiding this comment

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

image

Should the activation bar of the method (ParseAlertCommand()) end before the entire class returns?


The sequence diagram for the `AlertExpiringSoonCommand.run()` is shown below. The user can also call this via `alerts expiry`

![image](images/AlertExpirySequenceDiagram.png)

Choose a reason for hiding this comment

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

image

Same as above, should the activation bar end before the return statement?

For `AlertLowStockCommand`, it is less complicated, and the sequence diagram shown below. The user can also call this via `alerts stock`


![image](images/AlertStockSequenceDiagram.png)

Choose a reason for hiding this comment

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

image
image

Same as above, should the activation bar end before the return statement?

Choose a reason for hiding this comment

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

image

Should the activation bar not touch the frame of the loop?

The _sequence diagram_ below shows how the components interact with each other given a scenario where the user
enters the input `add n/carrot a/1 e/2021-11-12`

![image](images/InteractionSeqDiagram.png)

Choose a reason for hiding this comment

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

image

Should the activation bar be a full bar instead?

Copy link

@Roycius Roycius left a comment

Choose a reason for hiding this comment

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

DG is well done overall other than some potential improvements pointed out. Well done!


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Potential missing deletion of object:

  • AlertExpiringSoonCommand and AlertLowStockCommand when AlertCommand returns,
  • AlertCommand when Parser returns.


The class diagram below shows the relationships between `Parser`, `Command` and `XYZCommand`.

![image](images/ParserDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Suggestion: Add {abstract} to Command in the class diagram to make it clear that it is an abstract class.


### 3.1. System Architecture

![System Architecture](images/SysArch%20Diagram.png)
Copy link

Choose a reason for hiding this comment

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

Consider changing the colour choices for Parser and/or Command as the current colour choices may make it hard for readers to read the words inside the box.

Then calling `delete n/ carrot e/ 25/12/2021` will remove the second entry in the `carrot` category.
The sequence diagram below illustrates the above command example

![image](images/DeleteSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

The DeleteCommand instance in the diagram seems to not be referenced to anymore after Parser returns. Maybe add a "cross" here?
image


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

The activation bar should end when the method returns

46D56980-5F28-429A-8D03-92EB986DEDF0


The sequence diagram for the `AlertExpiringSoonCommand.run()` is shown below. The user can also call this via `alerts expiry`

![image](images/AlertExpirySequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

There should not be an activation bar after expiryDate is returned from Ingredient and before getIngredientInfo() is invoked.

Also, The activation bar for AlertExpiringSoonCommand should not exist after it returns.

81DBF483-F7AB-4223-90EE-4F53B0D8B131

For `AlertLowStockCommand`, it is less complicated, and the sequence diagram shown below. The user can also call this via `alerts stock`


![image](images/AlertStockSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Same as the previous diagram, the activation bar should not exist in ingredientGroup after totalAmount is returned and before toString() is called.

The activation bar for AlertExpiringSoonCommand is also too long.

D81FEC42-351A-4167-988F-E5BC7C2E9C5D


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Also, should there be a return from parseAlertCommand() method before the method called from "alerts all" return?

32883FE8-594F-412E-AB72-0800232C7F25


Below is a partial class diagram of the `IngredientList` component

![image](images/IngredientListDiagram.png)
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 missed some arrowheads in the diagram

9DA5FF6E-33A6-4C05-A757-296C9B055847

Copy link

@ruyian ruyian left a comment

Choose a reason for hiding this comment

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

Overall, a great work, team!

The _sequence diagram_ below shows how the components interact with each other given a scenario where the user
enters the input `add n/carrot a/1 e/2021-11-12`

![image](images/InteractionSeqDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Command's instantiated object is no longer used. An cross should be drawn at the end.


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

We need a cross for the instantiated AlertCommand once the parser returns as the instantiated object is no longer used.


The sequence diagram for when the user inputs `alerts all` is shown below.

![image](images/AlertsAllSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

The activation bar should have some difference in the length to show the execution flow


The sequence diagram for the `AlertExpiringSoonCommand.run()` is shown below. The user can also call this via `alerts expiry`

![image](images/AlertExpirySequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

The activation bar should not get cross the OPT of the flow diagram because it is not guaranteed to execute while only expireDate is guaranteed to be returned from Ingredient

The _sequence diagram_ below shows how the components interact with each other given a scenario where the user
enters the input `add n/carrot a/1 e/2021-11-12`

![image](images/InteractionSeqDiagram.png)

Choose a reason for hiding this comment

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

image

For this part, not very sure what "i" means here. Does it refer to the carrot that will be added to the list?


The **UI** component can be found in the `UI` package. The UI reads commands from the user, sends the command to `Main` to be executed and prints an output message upon completion of the command or if an error occurred.

### 3.3. Parser component

Choose a reason for hiding this comment

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

Instead of talking about how Parser calls the parseXYZCommand() and executes... in 3.4. Command component, will it be better to move the parseXYZCommand() part to 3.3. Parser component? Coz it is a method of the parser, and should be considered as part of the parser component I guess.


### 1.1. Purpose
This document specifies the architectural and software design decisions in the implementation of the Smart Inventory
Tracking and Updating System (SITUS).

Choose a reason for hiding this comment

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

Since there is "Instructions for manual testing" section in the DG, maybe can add additional purposes ( like showing the developers how to test/improve the software)?

`IngredientList` object will only use `loadIngredientsFromMemory()` and `writeIngredientsToMemory()` methods
of the storage class only when there is a change in the ingredient list of the program.

## 4. Implementation

Choose a reason for hiding this comment

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

For the implementation section, since this DG has one of the purposes as "specifying software design decisions", I guess it is good to also mention some of the alternatives designs your team had thought of, as well as their pros and cons, to explain why your team decided to use current design.

mudkip8 and others added 30 commits November 8, 2021 15:30
Trying to fix formatting issues for DG
Still trying to fix DG formatting
Third try's the charm for fixing DG formatting
Attempt 4 at fixing formatting
Revert "Attempt 4 at fixing formatting"
Attempt 5 at fixing DG formatting
Attempt 6 at fixing formatting issue DG
Attempt 7 at fixing DG formatting
Added some page breaks for DG PDF conversion
edit DG and AddCommandTest
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.

10 participants