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

[CS2103T-T10-2] DukeCooks #123

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

Conversation

bigjunnn
Copy link

@bigjunnn bigjunnn commented Oct 2, 2019


[discrete]
=== Use case: Delete person
=== Use case: Add recipe
Copy link

Choose a reason for hiding this comment

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

Consider numbering the use cases: e.g. Use case UC01: Add recipe

@@ -337,17 +339,115 @@ Use case ends.
* 3a. The given index is invalid.
Copy link

Choose a reason for hiding this comment

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

Should this be generalised such that it doesn't show information about UI?


[discrete]
=== Use case: Add Calorie Intake

Copy link

Choose a reason for hiding this comment

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

Maybe consider clarifying the information provided in Nutrition (Breakdown)? Its slightly unclear what the list of Nutrition means

+
Use case ends.

*Extensions*
Copy link

Choose a reason for hiding this comment

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

All your extensions are pretty exhaustive, good job!

* prefer desktop apps over other types
* can type fast
* prefers typing over mouse input
* is reasonably comfortable using CLI apps

*Value proposition*: manage contacts faster than a typical mouse/GUI driven app
*Value proposition*: monitors all aspects of health in one application

[appendix]
== User Stories

Choose a reason for hiding this comment

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

Have you considered adding more user stories? Could consider including additional stories that would be possible in v2.0

[none]
** 1a1. DukeCooks displays an error message.
+
Use case ends.

[appendix]
== Non Functional Requirements

Choose a reason for hiding this comment

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

Have you considered additional NFRs? For example, having backups in case of data corruption.


[appendix]
== Non Functional Requirements

. Should work on any <<mainstream-os,mainstream OS>> as long as it has Java `11` or above installed.
. Should be able to hold up to 1000 persons without a noticeable sluggishness in performance for typical usage.
. Should be able to hold up to 1000 recipes without a significant reduction in performance for typical usage.

Choose a reason for hiding this comment

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

Would you like to consider quantifying the "significant" reduction? This would help identifying whether the goal has been met.

The class diagram below gives an overview of the `Recipe` class.

.Recipe Class Diagram
image::RecipeClassDiagram.png[]

Choose a reason for hiding this comment

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

Hi would it be better if the UML diagram has only either associations via the arrows or associations via the attributes?


.Interactions Inside the Logic Component for the `delete 1` Command
.Interactions Inside the Logic Component for the `delete recipe 1` Command
image::DeleteSequenceDiagram.png[]

Choose a reason for hiding this comment

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

Would it be better if the sequence diagram had bigger font to allow the reader to see more easily?


The following sequence diagram shows how the undo operation works:
Step 2. The user executes `add task tn/bake a cake td/12/12/2019` command to add a new
task into Duke Cooks. The `add` command calls `Model#addDashboard()`, causing the task to

Choose a reason for hiding this comment

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

might want to consider splitting this into smaller steps because it looks quite cluttered to the reader in my opinion

Copy link

@hellodommy hellodommy left a comment

Choose a reason for hiding this comment

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

Good effort on the diagrams! 👍🏻


The following sequence diagram shows how the add operation works:

image::AddTaskSequenceDiagram.png[]

Choose a reason for hiding this comment

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

Inconsistent that the style of the diagram changed.

The following activity diagram summarizes what happens when a user executes a new command:
The Exercise class is represented by the following class diagram below.

image::ExerciseClassDiagram.png[]

Choose a reason for hiding this comment

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

Should choose to show association using attribute or with arrow, but not both.
There are also too many details in this diagram. Private attributions don't have to be shown. 🙈

alages97 added a commit to alages97/main that referenced this pull request Nov 1, 2019
Wrote tests and fixed corresponding bugs
bakwxh and others added 22 commits November 7, 2019 00:28
Create ViewMealPlan functionality
* 'master' of https://github.com/AY1920S1-CS2103T-T10-2/main:
  Write tests for MealPlan
  Write tests to improve coverage
  Fix EOF issue
  Fix UI for ViewMealPlan
  Add viewing of MealPlan
  Add EditPageCommand and corresponding tests
  Add styling to flowpane in DiaryCard
  Add more diary test cases
  Fix checkstyleTest
  Fix test cases
  Amend checkstyle
  Ammend result display for Recipe commands
  Make MealPlan creation and addition require existing Recipe
  Add more sample data to diary
  Add minor UI changes
  Add basic intro to PPP
  Write some StorageManager tests

# Conflicts:
#	src/test/java/dukecooks/logic/commands/CommandTestUtil.java
Add more test cases and checks to health records
� Conflicts:
�	src/test/java/dukecooks/storage/StorageManagerTest.java
� Conflicts:
�	src/main/java/dukecooks/logic/parser/ViewCommandParser.java
�	src/main/java/dukecooks/ui/MainWindow.java
Set to full screen window and fix sample data loading error
Add view workout functionality
yyuanxin and others added 30 commits November 11, 2019 22:55
Final updates to UG and DG
Update figure numbers in UG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants