Skip to content

[CS2113T-T09-4] Simplst#6

Open
LDerpy wants to merge 463 commits into
nus-cs2113-AY2122S2:masterfrom
AY2122S2-CS2113T-T09-4:master
Open

[CS2113T-T09-4] Simplst#6
LDerpy wants to merge 463 commits into
nus-cs2113-AY2122S2:masterfrom
AY2122S2-CS2113T-T09-4:master

Conversation

@LDerpy

@LDerpy LDerpy commented Mar 3, 2022

Copy link
Copy Markdown

Simplst is a laptop app for managing warehouse inventory and processes, optimized for use via a Command Line Interface (CLI) for warehouse management workers.

@khseah khseah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall good implementation! One more thing to consider is to put the UML diagrams on a new line. Some UML diagrams are right after the sentence, causing formatting issues

Comment thread docs/DeveloperGuide.md
#### Description
The goods class keeps track of the various inventory that will be input into the system through the commands class.
The diagram below shows the model component of the orderline class.
![Good Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/Good.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the UML format does not really follow the standards we learnt in lecture. Eg we use "+", "-", "#" or "~" to indicate the visibilities, but in the UML diagram shown it used a red square and green circle

Also the return type should be at the end instead. Eg:
getId(): Int
instead of:
int getId()

Comment thread docs/DeveloperGuide.md Outdated
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
### Good Class
#### Description
The goods class keeps track of the various inventory that will be input into the system through the commands class.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can consider highlighting important words to aid readability

Suggested change
The goods class keeps track of the various inventory that will be input into the system through the commands class.
The `goods` class keeps track of the various inventory that will be input into the system through the commands class.

Comment thread docs/DeveloperGuide.md Outdated
### Regex Class
#### Description
The Regex Class is created to simplify the use of regular expressions in Java.
![Regex Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/Regex.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This diagram indicates that a new Regex class is instantiated and the red marking shows that the activation bar is a constructor, which is not the case. You should add a dotted line before the activation bar
Screenshot 2022-03-31 101327

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Return arrows should be dotted

Comment thread docs/DeveloperGuide.md

## Design & implementation

## Design

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can consider giving a high level overview of the architecture first, before diving deep into explaining each detail

Comment thread docs/DeveloperGuide.md
#### Description
The goods class keeps track of the various inventory that will be input into the system through the commands class.
The diagram below shows the model component of the orderline class.
![Good Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/Good.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding a <br> to move this diagram to a new line.
image

Comment thread docs/DeveloperGuide.md
![Regex Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/Regex.puml)
The Class Diagram above shows the different variables and methods associated with the regex class.
This class takes in the regular expression as a string and the string to be matched in the class contructor, and stores the results
in the private variable `groupValues` which is a Hash Map where the key would be the capture group name and the value as the matched results.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider using HashMap in a code block instead of Hash Map to make it clear that it is a class.
image

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider making the return lines dotted lines.
image

Comment thread docs/DeveloperGuide.md Outdated
3.1.1 Lastly, the list of orders is printed to the command line.
3.2 Else, if the flag is `g/`, the User Interface Class will call the listGoods() method in the Warehouse class
3.2.1 Following that, the Warehouse class will call the getGoods() method on the Good class
3.2.2 Lastly, the list of goods is printed to the command line.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding <br> lines to properly format these numbered lists, as they currently appear as one big paragraph.
image

Comment thread docs/DeveloperGuide.md Outdated
4. This method will then iterate through orderLists and sum up each quantity of each Good object in each order.
5. The totalGoods() method will return an integer of the number of goods in the warehouse.

For more examples of how a user can use a command, refer to the [UserGuide](/UserGuide.md)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This line seems unnecessary, but it could be more useful if it linked to this command's section specifically in the User Guide. Consider changing it or removing it.
image

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

User is not a class, so the colon before User should not be included. Consider removing it.
image

Comment thread docs/DeveloperGuide.md Outdated

### View Good Method
#### Description
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be good to show the command format separately in text form as a code block, as well as an example. They are currently only visible in the sequence diagrams which makes it hard to copy and paste them, and is less obvious to the reader.

Comment thread docs/DeveloperGuide.md Outdated
3. The Regex Class will then be called to match the rest of the user's input to find the values required to remove goods from the inventory.
4. Afterwards, the User Interface class will call removeGood() method from the Commands Class
5. This method will then reduce the quantity of a type of goods if the quantity input is not larger than the existing quantity. If the quantity input is the same as the existing quantity, the goods object will be removed from the inventory.
6. The UI will show a message of format

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider including more details on what methods are called and what types of objects are returned by each method, to make the implementation of this program clearer.

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The stickman figures are not covered in this module, so consider replacing them with boxes.
image

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The boxes + User at the bottom should not be present. Use an X instead to mark end of life for an object instead if necessary, or leave it empty.
image

Comment thread docs/DeveloperGuide.md Outdated
### Good Class
#### Description
The goods class keeps track of the various inventory that will be input into the system through the commands class.
The diagram below shows the model component of the orderline class.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An orderline class is mentioned here, but there is no prior explanation or link to this class beforehand.

Comment thread docs/DeveloperGuide.md
Comment on lines +38 to +46
```
String regex = "id/(?<id>\\d*) n/(?<name>.*) q/(?<qty>\\d*) d/(?<desc>\\.*)";
Regex regexMatch = new Regex(userInput, regex);
HashMap<String, String> matches = regexMatch.getGroupValues();
String id = matches.get("id");
String name = matches.get("name");
String quantity = matches.get("qty");
String description = matches.get("desc");
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good that an example is given on how it works, but perhaps some explanation could be given for the regex pattern?

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +113 to +118
3. Together with the previous step, the User Interface Class will check the flag followed by `list`
3.1 If the flag is `o/`, the User Interface Class will call the listOrders() method in the Warehouse class
3.1.1 Lastly, the list of orders is printed to the command line.
3.2 Else, if the flag is `g/`, the User Interface Class will call the listGoods() method in the Warehouse class
3.2.1 Following that, the Warehouse class will call the getGoods() method on the Good class
3.2.2 Lastly, the list of goods is printed to the command line.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe some formatting error here!

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +48 to +52
#### Description
The Commands Class is the class which contains all the available commands that can be used in the Simplst CLI application.
![Commands Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/CommandsClass.puml)

More about how each feature is run can be seen in each of the methods that can be found below.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

perhaps an explanation can be given for why this design of putting all commands into a single class was chosen rather than subclassing different commands.

@Lim-Shi-Han Lim-Shi-Han left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good in general, slight changes to the formatting and diagrams would make it better!:)

Comment thread docs/DeveloperGuide.md Outdated
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
### Good Class
#### Description
The goods class keeps track of the various inventory that will be input into the system through the commands class.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes as @khseah said, I also think denoting the class names as code would be good. In addition I think it would be better to capitalise it as well (e.g. Good)

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +97 to +98
> `quantity` `name` have been removed
to show that the operation is successful.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The formatting could be improved here

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +107 to +108
![List sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/List.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The activation bar for the first condition should not spill over into the "else" condition

image

Comment thread docs/DeveloperGuide.md Outdated
### Regex Class
#### Description
The Regex Class is created to simplify the use of regular expressions in Java.
![Regex Class Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/Regex.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think there might be some issues with the dummyClass calling Regex, as it should be a new activation bar (and the previous activation bar should have ended prior).

image

Comment thread docs/DeveloperGuide.md Outdated
View Good belongs as part of the Commands Class. It is used when a user would like to know more information about an inventory item has an item id linked to it in the warehouse.

#### Operation
![View Good Sequence Diagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/AY2122S2-CS2113T-T09-4/tp/master/docs/diagrams/viewGood.puml)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Return lines should be dotted.

LDerpy and others added 30 commits April 11, 2022 23:33
# Conflicts:
#	src/main/java/seedu/simplst/Simplst.java
…into branch-Shyunyin-2101UG1

# Conflicts:
#	docs/UserGuide.md
Revert "Update StorageTest.java"
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.

9 participants