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

[CS2113-F14-3] BYTE-CEPS #53

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

Conversation

joshualeejunyi
Copy link

BYTE-CEPS is a fitness application that allows tech-savvy users of all kind keep track of their fitness journey through the simplicity and efficiency of the command line interface.

@joshualeejunyi joshualeejunyi changed the title [F14-3] BYTE-CEPS [CS2113-F14-3] BYTE-CEPS Mar 8, 2024
yeozongyao pushed a commit to yeozongyao/tp that referenced this pull request Apr 1, 2024
Copy link

@NgYaoDong NgYaoDong left a comment

Choose a reason for hiding this comment

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

Generally good job, just minor issues here and there. :)

Comment on lines 55 to 56
![addExercise](https://github.com/V4Vern/tp/assets/28131050/45f7e9b3-8a31-4dfe-a783-433acb71fa58)

Choose a reason for hiding this comment

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

Should there be an extra box coming out since you are calling a method of its own class?
image

Comment on lines 55 to 56
![addExercise](https://github.com/V4Vern/tp/assets/28131050/45f7e9b3-8a31-4dfe-a783-433acb71fa58)

Choose a reason for hiding this comment

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

Should the activation bar for ExerciseManager be extended all the way to where it says "Print error message indicating invalid command format"?
image

Choose a reason for hiding this comment

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

Assuming the print success message is void, the arrow should be dashed.

image


The sequence diagram below shows how a log is created.

![WorkoutLogOverview.png](./diagrams/WorkoutLogOverview.png)


## Product scope

Choose a reason for hiding this comment

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

Target user profile and value proposition seems to be missing.

### `Activity` and child classes
The `Activity` class serves as a parent class to `Exercise`, `ExerciseLog`, `Workout`, `WorkoutLog` and `Day` classes for the ease of usage of `ActivityManager` classes (see below).

![ActivityClassDiagram](diagrams/ActivityClassDiagram.png)

Choose a reason for hiding this comment

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

exerciseLogs and exerciseList's visibilities seem to be missing?
image

### `Activity` and child classes
The `Activity` class serves as a parent class to `Exercise`, `ExerciseLog`, `Workout`, `WorkoutLog` and `Day` classes for the ease of usage of `ActivityManager` classes (see below).

![ActivityClassDiagram](diagrams/ActivityClassDiagram.png)

Choose a reason for hiding this comment

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

Is the Parser class missing an activation bar?
image

### `Activity` and child classes
The `Activity` class serves as a parent class to `Exercise`, `ExerciseLog`, `Workout`, `WorkoutLog` and `Day` classes for the ease of usage of `ActivityManager` classes (see below).

![ActivityClassDiagram](diagrams/ActivityClassDiagram.png)

Choose a reason for hiding this comment

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

Should there be a C logo beside the name of your classes?
image

Copy link

@nkotaa nkotaa left a comment

Choose a reason for hiding this comment

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

Somewhat verbose.

Comment on lines 29 to 42
+ execute(parser : Parser) : String
- executeInfoAction(parser : Parser) : String
- executeListAction(parser : Parser) : String
- executeUnassignAction(parser : Parser) : String
- executeAssignAction(parser : Parser) : String
- executeDeleteAction(parser : Parser) : String
- executeCreateAction(parser : Parser) : String
- processWorkout(parser : Parser) : Workout
- assignExerciseToWorkout(parser : Parser) : String
- getFullWorkoutString(workoutPlanName : String) : String
- unassignExerciseFromWorkout(parser : Parser) : String
+ getActivityType(plural : boolean) : String
- executeSearchAction(parser : Parser) : String
}
Copy link

Choose a reason for hiding this comment

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

Note that not all attributes and methods need be stated. Consider including only the relevant ones.

Comment on lines 60 to 83
+ {static} DAYS : String[]
- exerciseManager : ExerciseManager
- workoutManager : WorkoutManager
- workoutLogsManager : WorkoutLogsManager
+ WeeklyProgramManager(exerciseManager : ExerciseManager, workoutManager : WorkoutManager, workoutLogsManager : WorkoutLogsManager)
- initializeDays() : void
- getDay(day : String) : Day
+ execute(parser : Parser) : String
- executeListAction() : String
- executeAssignAction(parser : Parser) : String
+ assignWorkoutToDay(workout : Activity, day : String) : String
- getDayFromDate(date : LocalDate) : Day
- getDayFromDate(dateString : String) : Day
- executeLogAction(parser : Parser) : String
- {static} getWorkoutName(selectedDay : Day, workoutDate : String) : String
- executeTodayAction() : String
- getTodaysWorkoutString(givenWorkout : Workout, workoutDate : String, workoutDay : Day) : String
- executeHistoryAction(parser : Parser) : String
- getHistoryString() : String
- executeClearAction(parser : Parser) : String
+ exportToJSON() : JSONObject
+ getActivityType(plural : boolean) : String
+ getListString() : String
}
Copy link

Choose a reason for hiding this comment

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

As above.

Comment on lines 14 to 24
#### The `Exercise` class
An `Exercise` object represents a single exercise entered by the user. The exercise name is stored in `Exercise.activityname`.

#### The `Workout` class
A `Workout` object represents a single workout created by the user. It contains an `ArrayList` of `Exercise` objects.

#### The `ExerciseLog` class
An `ExerciseLog` object is similar to an `Exercise` object, except that it also contains information on the weight, sets and repetitions of each exercise.

#### The `WorkoutLog` class
A `WorkoutLog` object is similar to that of a `Workout` object, except it contains an `ArrayList` of `ExerciseLog` objects.
Copy link

Choose a reason for hiding this comment

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

Would all this not already be self-evident from the class diagram?

Comment on lines 79 to 83
1. The process begins with the user inputting a command via the command-line interface. In this scenario, the User provides the command `exercise /list` to list all exercises.
2. The command parser receives the user input and parses it to extract relevant information, such as the action (`exercise /list`) and any additional parameters.
3. Upon receiving the parsed command, the Exercise Manager component validates the input to ensure it conforms to the expected format and criteria. This step is crucial for maintaining data integrity and preventing errors in subsequent processing.
4. If the input passes validation, the ExerciseManager instructs the ActivityManager to list all exercises. The ActivityManager retrieves the list of exercises from the activitySet. The ActivityManager returns the exercise list to the ExerciseManager. The ExerciseManager prints the list of exercises to the User.
5. If the input fails validation, an error message is generated and displayed to the user, informing them of the invalid command format. This ensures that users receive timely feedback and can correct their input accordingly
Copy link

Choose a reason for hiding this comment

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

Consider starting the sequence diagram at a later stage, say when the command has already been parsed, for a more condensed and meaningful explanation.


The sequence diagram below shows how an exercise is created.

![addExercise](https://github.com/V4Vern/tp/assets/28131050/45f7e9b3-8a31-4dfe-a783-433acb71fa58)
Copy link

Choose a reason for hiding this comment

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

FYI you should link to repository resources instead of external links, as done above.


The sequence diagram below shows how an exercise is deleted.

![deleteExercise](https://github.com/V4Vern/tp/assets/28131050/3fde6b4e-d292-497a-9468-2118125678a7)
Copy link

Choose a reason for hiding this comment

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

Kindly add activation boxes for self-method calls in the ExerciseManager.

Copy link

@wjunjie01 wjunjie01 left a comment

Choose a reason for hiding this comment

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

Overall looks good except for some minor diagram issue

@@ -4,9 +4,118 @@

{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}

## Design & implementation
## Classes: overview

Choose a reason for hiding this comment

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

Would it be better to include a table of contents (with links to the relevant sections) for easier navigability?

### `Activity` and child classes
The `Activity` class serves as a parent class to `Exercise`, `ExerciseLog`, `Workout`, `WorkoutLog` and `Day` classes for the ease of usage of `ActivityManager` classes (see below).

![ActivityClassDiagram](diagrams/ActivityClassDiagram.png)

Choose a reason for hiding this comment

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

Should there be a 'C' symbol in the class diagram?

Choose a reason for hiding this comment

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

Overall the class diagram seems quite comprehensive covering the relevant classes. Nice.

4. <code>getListString()</code>: Get the string containing all the activities contained in the <code>ActivityManager</code>.
5. `execute()`: Execute all commands related to the `ActivityManager` and return the required user input.

![ActivityManagerClassDiagram](diagrams/ActivityManagerClassDiagram.png)

Choose a reason for hiding this comment

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

Should there be a 'C' / 'A' symbol in the class diagram?

Comment on lines 75 to 77

The sequence diagram below shows how an exercise is edited.

Choose a reason for hiding this comment

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

It seems that there is a missing sequence diagram here?


The sequence diagram below shows how an exercise is created.

![addExercise](https://github.com/V4Vern/tp/assets/28131050/45f7e9b3-8a31-4dfe-a783-433acb71fa58)

Choose a reason for hiding this comment

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

Should the return arrow in the sequence be depicted as a solid line?

Choose a reason for hiding this comment

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

Can the invocation of the method be more concise?

Choose a reason for hiding this comment

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

Are there supposed to be two class boxes at the top and bottom?


The sequence diagram below shows how exercises can be listed

![listExercise](https://github.com/V4Vern/tp/assets/28131050/eebe0e2e-d486-4644-b36d-e26b499d1f53)

Choose a reason for hiding this comment

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

Should the return arrow in the sequence be depicted as a solid line?

Choose a reason for hiding this comment

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

Is an activation bar required when invoking a method within the function?


The sequence diagram below shows how an exercise can be searched.

![searchExercises](https://github.com/V4Vern/tp/assets/28131050/fd32eba4-a7f1-460d-81cc-d9e57ca100c2)

Choose a reason for hiding this comment

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

Is an activation bar required when invoking a method within the function?


The sequence diagram below shows how an exercise is deleted.

![deleteExercise](https://github.com/V4Vern/tp/assets/28131050/3fde6b4e-d292-497a-9468-2118125678a7)

Choose a reason for hiding this comment

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

Is an activation bar required when invoking a method within the function?

### `Activity` and child classes
The `Activity` class serves as a parent class to `Exercise`, `ExerciseLog`, `Workout`, `WorkoutLog` and `Day` classes for the ease of usage of `ActivityManager` classes (see below).

![ActivityClassDiagram](diagrams/ActivityClassDiagram.png)

Choose a reason for hiding this comment

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

Consider omitting methods that are trivial/unnecessary to improve the readability of the class diagram.

Comment on lines 55 to 56
![addExercise](https://github.com/V4Vern/tp/assets/28131050/45f7e9b3-8a31-4dfe-a783-433acb71fa58)

Choose a reason for hiding this comment

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

Assuming the print success message is void, the arrow should be dashed.

image

|--------|----------|---------------|------------------|
|v1.0|new user|see usage instructions|refer to them when I forget how to use the application|
|v2.0|user|find a to-do item by name|locate a to-do without having to go through the entire list|
| Version | As a ... | I want to ... | So that I can ... |

Choose a reason for hiding this comment

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

Consider adding more user stories

joshualeejunyi and others added 30 commits April 15, 2024 18:51
Add key UG grammar corrections
Update readme.md and Fix table
Actually fix table insha'allah
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.

8 participants