[CS2113-F13-1] Taste of Mom's by Aung-Phone-Naing · Pull Request #49 · nus-cs2113-AY2223S2/tp

Aung-Phone-Naing · 2023-03-03T05:22:31Z

Taste of Mom's helps students manage recipes, optimized for use via a Command Line Interface. This ensures that frequent tasks can be done faster by typing in commands.

Jeraldchen · 2023-03-29T05:23:01Z

+**Main components of the architecture**
+
+![image](./PlantUML/MainArchitecture.png)
+


Architecture diagram is clear and simple to understand.

Jeraldchen · 2023-03-29T05:27:45Z

+The **API** of this component is specified in [`Parser.java`](https://github.com/AY2223S2-CS2113-F13-1/tp/blob/master/src/main/java/seedu/duke/parser/Parser.java)
+
+![image](./PlantUML/ParserComponent.png)
+


Remove the C and I symbols beside the Class and Interface boxes as it is not the module convention. Replace the private and public access modifiers with - and + symbols instead.

Jeraldchen · 2023-03-29T05:29:35Z

+The **API** for this component is specified in [`RecipeList.java`](https://github.com/AY2223S2-CS2113-F13-1/tp/blob/master/src/main/java/seedu/duke/recipe/RecipeList.java)
+
+![RecipeList Component](./PlantUML/RecipeListComponent.png)
+


Remove the C symbols beside the Class boxes as it is not the module convention. Replace the private and public access modifiers with - and + symbols instead.

Jeraldchen · 2023-03-29T05:31:26Z

+
+> The following sequence diagram shows how the help feature works:
+![Sequence Diagram for Help](./PlantUML/Help.png)
+


The class boxes at the bottom of your diagrams should be removed.

Jeraldchen

Overall, DG is clear and well done. There are some common syntax errors in the sequence diagrams that do not adhere to module standards as mentioned in the other comments.

Brennanzuz

Overall many many examples and code snippets to extensively show what the user can input, what is expected, and the reason for the implementation as a bonus. Ample number of diagrams to really illustrate the flow, even with a huge diagram to tie everything together. Besides understandably not adhering to the format for the class diagrams, the developer guide is very comprehensive and helpful.

Brennanzuz · 2023-03-29T05:39:40Z

I like how an example of just one command is given (delete 1) as it helps to understand the general flow of the program while keeping the diagram simple and readable. This is provided that delete 1 is the most exhaustive example that shows the most classes involved.

Brennanzuz · 2023-03-29T05:41:06Z

@@ -0,0 +1,26 @@
+@startuml
+


A common problem among all groups including mine is to have the little icons. You can add:

hide circle skinparam classAttributeIconSize 0

to remove them and follow the recommended format.

Brennanzuz · 2023-03-29T05:46:10Z

Quotation marks around functions can be removed. I also like the mindful deletion of only the :Command object as the other classes will continue to be used in other functions, while the :Command object will no longer be used.

Brennanzuz · 2023-03-29T05:48:01Z

Can the multiplicity of Step be specified? If it can, then it would be good to indicate.

KedrianLoh

Well done overall!

KedrianLoh · 2023-03-30T03:11:07Z

The dotted line (delete 1) between UI and Duke seems a little ambiguous.

KedrianLoh · 2023-03-30T03:28:41Z

Since the HELP message shown does not require any further user input, consider changing it to a dotted line. The UI can be shifted to the rightmost portion of the sequence diagram since it is only used after Command.

Modify UG to match actual output

jeyvia · 2023-04-01T03:24:56Z

+        }
+        CommandType type;
+        switch (lineSpaced[0]) {
+        case "list":


For the switch-case, do change the magic strings in the case to constant variables, like you have done so in your Command.java.

jeyvia · 2023-04-01T03:30:47Z

+        parsed.add(parsedTagStep[0].trim());
+        try {
+            parsed.add(parsedTagStep[1].trim());
+        }catch (Exception e){


I'm not sure how the checker did not pick up on this, but there a code-style violation here. Do check your code thoroughly for these issues :)

jeyvia · 2023-04-01T03:34:52Z

+
+

Do keep the spacing between methods consistent as well! Some have 1 or 2 lines, some have no lines at all.

jeyvia · 2023-04-01T03:38:35Z

+     * @throws IncompleteInputException if full description input is missing the description or due date or both.
+     */
+    public static ArrayList<String> parseRecipe(String description) throws IncompleteInputException {
+        ArrayList<String> parsed = new ArrayList<>();


You could follow more conventional naming for this variable. To my understanding these are your parsed command parameters, perhaps something a little more descriptive? :)

jeyvia · 2023-04-01T03:45:34Z

+    public void showInvalidStepMessage() {
+        System.out.println(StringLib.INVALID_STEP);
+    }
+}


Good job minimising the use of magic strings here! But do consider changing as many magic strings to constants as possible!

jeyvia · 2023-04-01T03:50:34Z

+    public ArrayList<Step> getList() {
+        return stepList;
+    }
+


Well done with the method naming conventions!

jeyvia · 2023-04-01T03:54:40Z

+    private static boolean matchString(String input, String regex) {
+        String matcher = input;
+        int count = 0;
+        while (matcher.contains(regex)){


There are violations with regards to spacing here as well, do keep a look out for these!

… ensure detectability

…hat starts with a vowel.

…ng/tp into PE-D-Fixes-Branch

PE-D bug fixes

Fix Issues #92, #102, #103

Follow-Up on #107

# Conflicts: # data/1-Hotpot.txt # src/main/java/seedu/duke/ui/StringLib.java

Fix lay out bugs in PPP

grammar check for recipelist

Step List Grammar update

Fixes DG bugs on github pages

UG grammar fixed

Final UG and DG format fixes

PPP finalised for hongyao

Update YatPang's PPP

Final PPP Update

Jeraldchen reviewed Mar 29, 2023

View reviewed changes

Brennanzuz reviewed Mar 29, 2023

View reviewed changes

KedrianLoh reviewed Mar 30, 2023

View reviewed changes

Merge pull request #63 from YatPang/master

8dd61fa

Modify UG to match actual output

jeyvia reviewed Apr 1, 2023

View reviewed changes

Aung-Phone-Naing and others added 21 commits April 1, 2023 16:45

Strips all leading spaces and converts command words to lower case to…

82feaef

… ensure detectability

Adds link tto Jar file

1529e90

Fixes minor gramatical error when viewing some dishes with a flavor t…

38f0529

…hat starts with a vowel.

Merge branch 'master' into PE-D-Fixes-Branch

ae028cb

Fixes expected output for viewing recipe

ad1a983

Merge branch 'PE-D-Fixes-Branch' of https://github.com/Aung-Phone-Nai…

d5bfaeb

…ng/tp into PE-D-Fixes-Branch

Merge pull request #106 from Aung-Phone-Naing/PE-D-Fixes-Branch

4b7abc5

PE-D bug fixes

Fix Issues #92, #102, #103

758a79e

Fix Issues #92, #102, #103

287f24d

Fix Issues #92, #97, #102, #103

d167880

Fetch add edit feature

dbcd99f

Fix Issues #69, #92, #97, #102, #103

1a7f3a4

Fix Issues #69, #92, #97, #102, #103

698397e

Fix Issues #69, #92, #97, #102, #103

268fd28

Fix Issues #69, #92, #97, #102, #103

ff3d52b

Merge pull request #107 from YatPang/master

4ba790f

Fix Issues #92, #102, #103

Follow-Up on #107

13291d7

Follow-Up on #107

043e560

Merge pull request #108 from YatPang/master

bdd960a

Follow-Up on #107

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-1/tp

76007db

# Conflicts: # data/1-Hotpot.txt # src/main/java/seedu/duke/ui/StringLib.java

Fetch: Add edit method

d00512d

liuziyang020319 and others added 30 commits April 10, 2023 15:39

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-1/tp

a6c94ac

Merge remote-tracking branch 'upstream/master'

15ee28b

Fix grammar bugs in log

7a873fd

grammar check for recipelist

bbdb20c

Merge pull request #145 from liuziyang020319/master

b71c052

Fix lay out bugs in PPP

Change StepsList Grammar

2982656

fixed grammar for add and delete commands

ce3e9f4

Merge pull request #146 from LimHongYao/master

7cb803d

grammar check for recipelist

Fixes step delete quit message grammar

8263c28

Merge branch 'AY2223S2-CS2113-F13-1:master' into master

cc032cd

Merge pull request #147 from Aung-Phone-Naing/master

ef76dd8

Step List Grammar update

UG grammar fixed

91ef522

Fixes DG bugs on github pages

dba1b33

Merge pull request #149 from Aung-Phone-Naing/master

62c1317

Fixes DG bugs on github pages

Merge pull request #148 from liuziyang020319/master

8b06c76

UG grammar fixed

Final UG format fixes

2874a6a

Final DG format fixes

548bd7d

Merge pull request #150 from Aung-Phone-Naing/master

eb4f05c

Final UG and DG format fixes

PPP finalised

6677071

Merge branch 'AY2223S2-CS2113-F13-1:master' into master

5550e66

Merge pull request #151 from LimHongYao/master

8924ab7

PPP finalised for hongyao

Update YatPang's PPP

b43792c

Merge pull request #152 from YatPang/master

aedeef7

Update YatPang's PPP

Adds Phone's PPP

79fd1cb

Merge branch 'AY2223S2-CS2113-F13-1:master' into master

1ff4e21

Merge pull request #153 from Aung-Phone-Naing/master

8ebb4bb

Update PPP

f3fd69d

Merge branch 'AY2223S2-CS2113-F13-1:master' into master

daadd13

Adds more PRs to PRs reviewed for PPP

71ab057

Merge pull request #154 from Aung-Phone-Naing/master

197e200

Final PPP Update

		Main components of the architecture

		![image](./PlantUML/MainArchitecture.png)

		The API of this component is specified in [`Parser.java`](https://github.com/AY2223S2-CS2113-F13-1/tp/blob/master/src/main/java/seedu/duke/parser/Parser.java)

		![image](./PlantUML/ParserComponent.png)

		The API for this component is specified in [`RecipeList.java`](https://github.com/AY2223S2-CS2113-F13-1/tp/blob/master/src/main/java/seedu/duke/recipe/RecipeList.java)

		![RecipeList Component](./PlantUML/RecipeListComponent.png)


		> The following sequence diagram shows how the help feature works:
		![Sequence Diagram for Help](./PlantUML/Help.png)

Uh oh!

Conversation

Aung-Phone-Naing commented Mar 3, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jeraldchen left a comment

Choose a reason for hiding this comment

Uh oh!

Brennanzuz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KedrianLoh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants