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-W16-2] ZeroToOne #11

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

Conversation

jiachen247
Copy link

@jiachen247 jiachen247 changed the title [CS2103-W16-2] AkshayFit [CS2103-W16-2] ZeroToOne Mar 4, 2020
@jiachen247 jiachen247 changed the title [CS2103-W16-2] ZeroToOne [CS2103T-W16-2] ZeroToOne Mar 4, 2020
+

* 2b. There are no exercises in the workout
** 2b1. System shows a message to inform the user

Choose a reason for hiding this comment

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

2b1. Consider prompting the user to create the new non-existent workout.

Copy link

@Permas-Teo Permas-Teo left a comment

Choose a reason for hiding this comment

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

Good job!


[NOTE]
As a more OOP model, we can store a `Tag` list in `Address Book`, which `Person` can reference. This would allow `Address Book` to only require one `Tag` object per unique `Tag`, instead of each `Person` needing their own `Tag` object. An example of how such a model may look like is given below. +
As a more OOP model, we can store a `Tag` list in `Zero To One`, which `Exercise` can reference. This would allow `Zero To One` to only require one `Tag` object per unique `Tag`, instead of each `Exercise` needing their own `Tag` object. An example of how such a model may look like is given below. +
Copy link

Choose a reason for hiding this comment

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

This doesn't make sense. Please check!

@@ -117,17 +117,21 @@ NOTE: The lifeline for `DeleteCommandParser` should end at the destroy marker (X
.Structure of the Model Component
image::ModelClassDiagram.png[]

*API* : link:{repoURL}/src/main/java/seedu/address/model/Model.java[`Model.java`]
*API* : link:{repoURL}/src/main/java/seedu.zerotoone/model/Model.java[`Model.java`]
Copy link

Choose a reason for hiding this comment

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

Link is broken!

@@ -117,17 +117,21 @@ NOTE: The lifeline for `DeleteCommandParser` should end at the destroy marker (X
.Structure of the Model Component
image::ModelClassDiagram.png[]
Copy link

Choose a reason for hiding this comment

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

I don't see the ExerciseManager, WorkoutManager etc. in your code. Will they be implemented? If not, shouldn't they not appear in this diagram?

Copy link

Choose a reason for hiding this comment

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

Model doesn't seem to be extending all the interfaces shown, why the discrepancy?

Copy link

Choose a reason for hiding this comment

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

Please check this diagram thoroughly and re-do one by looking at your implementation.

@@ -137,76 +141,76 @@ image:BetterModelClassDiagram.png[]
.Structure of the Storage Component
image::StorageClassDiagram.png[]

*API* : link:{repoURL}/src/main/java/seedu/address/storage/Storage.java[`Storage.java`]
*API* : link:{repoURL}/src/main/java/seedu.zerotoone/storage/Storage.java[`Storage.java`]
Copy link

Choose a reason for hiding this comment

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

Broken link!

@@ -137,76 +141,76 @@ image:BetterModelClassDiagram.png[]
.Structure of the Storage Component
image::StorageClassDiagram.png[]
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 missed the WorkoutListStorage, ScheduleListStorage etc. Please add them in.


==== Implementation

image::WorkoutSessionDiagram.png[]
Copy link

Choose a reason for hiding this comment

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

PlantUML by default uses square/circle/triangle to indicate visibility and symbols such as C, E for classes, enumerations etc. That is not standard UML, and needs to be changed. Please refer to PlantUML's documentation on how to do so.


*Value proposition*: manage contacts faster than a typical mouse/GUI driven app
*Value proposition*:
Manage workouts faster than a typical mouse/GUI-driven app

[appendix]
== User Stories
Copy link

Choose a reason for hiding this comment

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

Make sure all user stories are grammatically correct. I see some inconsistencies.

As a user......., I want TO ......., so that I can .......

* 2b. There are no exercises in the workout
** 2b1. System shows a message to inform the user
+
Use case resumes at step 2
Copy link

Choose a reason for hiding this comment

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

Won't this get stuck up in a loop?

It should instead resume at step 3.

A contact detail that is not meant to be shared with others
[[exercise]]Exercise::
A single type of exercise, for example push ups or crunches.
GUI
Copy link

Choose a reason for hiding this comment

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

This line gets concatenated to the previous one.

. A user with above average typing speed for regular English text (i.e. not code, not system admin commands) should be able to accomplish most of the tasks faster using commands than using the mouse.

_{More to be added}_
. Should not depend on a remote server, so that a user can exercise in any condition or environment.

[appendix]
== Glossary
Copy link

Choose a reason for hiding this comment

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

Would be better if you could add Workout and Session as well and any other terms you feel are relevant.

alloystory and others added 30 commits April 13, 2020 21:50
* edit About Us and User Guide

* edit jar build file

* edit compeltedExercise to session

* align workout to top

* fix bug in exercise

* edit add command test
* Add logging to Workout Parser classes

* Logging for workout commands

* Add logging to workout model

Co-authored-by: Guofeng <[email protected]>
* simulate randomness in sample logs

* fix broken link in DG

* add PPP

Co-authored-by: Guofeng <[email protected]>
Add logging, update DG and PPP
Update user feedback messages and fix workout name validation
* Edit

* PR+

* Edit PPP for stephen

* Edit PPP for stephen

Co-authored-by: gb3h <[email protected]>
Rename PPP page filename to all lowercase
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.

7 participants