Skip to content

[CS2113-W14-3] FitChasers#9

Open
ZhongBaode wants to merge 625 commits into
nus-cs2113-AY2526S1:masterfrom
AY2526S1-CS2113-W14-3:master
Open

[CS2113-W14-3] FitChasers#9
ZhongBaode wants to merge 625 commits into
nus-cs2113-AY2526S1:masterfrom
AY2526S1-CS2113-W14-3:master

Conversation

@ZhongBaode
Copy link
Copy Markdown

@ZhongBaode ZhongBaode commented Sep 17, 2025

FitChaser is a Command Line Interface (CLI)-based fitness tracking app designed for NUS
gym-goers. It is optimized for CLI users so that frequent tasks can be done faster by typing in commands

@ZhongBaode ZhongBaode changed the title [CS2113-W14-3] [CS2113-W14-3] FitChasers Sep 17, 2025
danielkwan2004 pushed a commit to danielkwan2004/tp that referenced this pull request Oct 14, 2025
Copy link
Copy Markdown

@NigelYeoTW NigelYeoTW left a comment

Choose a reason for hiding this comment

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

Overall lgtm, good progress, keep it up 🔥

*/
public Person(String name) {
setName(name); // reuse setter để kiểm tra name
this.weightHistory = new ArrayList<>();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any reason why the variable is not instantiated during declaration?

* @throws IllegalArgumentException if name is null or empty
*/
public Person(String name) {
setName(name); // reuse setter để kiểm tra name
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comments should be in as per coding standards

*
* @return true if a record was removed, false if no records exist
*/
public boolean removeLatestWeightRecord() {
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 could make the method sound more like a boolean

*/
public Exercise(String name, int reps) {
this.name = name;
this.sets = new ArrayList<>();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any reason why this is not initialised on declaration

public Exercise(String name, int reps) {
this.name = name;
this.sets = new ArrayList<>();
this.sets.add(reps);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any reason why you did you just use your defined method to add reps for the class?

Comment on lines +48 to +55
if(dateStr.isEmpty()) {
dateStr = LocalDate.now().format(DateTimeFormatter.ofPattern("dd/MM/yy"));
ui.showMessage("You missed out the date! Using current date: " + dateStr);
}
if(timeStr.isEmpty()) {
timeStr = LocalTime.now().format(DateTimeFormatter.ofPattern("HHmm"));
ui.showMessage("You missed out the time! Using current time: " + timeStr);
}
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 ensure whitespace consistency

Comment on lines +215 to +222
ui.showMessage("No workouts recorded yet!");
return;
}

for (int i = 0; i < workouts.size(); i++) {
Workout w = workouts.get(i);
ui.showMessage("------------------------------------------------");
ui.showMessage("[" + (i + ARRAY_OFFSET) + "]: " + w.getWorkoutName() +
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 UI can have functions to output these?

Comment on lines +225 to +237
if (w.getExercises().isEmpty()) {
ui.showMessage(" No exercises added yet.");
} else {
for (int j = 0; j < w.getExercises().size(); j++) {
Exercise ex = w.getExercises().get(j);
ui.showMessage(" Exercise " + (j + 1) + ". " + ex);
for (int k = 0; k < ex.getSets().size(); k++) {
ui.showMessage(" Set " + (k + 1) + " -> Reps: " + ex.getSets().get(k));
}
}
}
}
}
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 try to reduce nesting for readability

Comment on lines +274 to +275
boolean usedDefaultDate = false;
boolean usedDefaultTime = false;
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 variables can sound more like booleans

* @param scanner Scanner for reading user input in the retry loop
* @param initialArgs Initial command arguments containing end date/time details
*/
public void endWorkout(Scanner scanner, String initialArgs) {
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 refactor this methods as it seems to be quite long, the logic paths could also be made more readable

Comment thread docs/DeveloperGuide.md Outdated
Given below is a quick overview of main components and how they interact with each other.

#### Main components of the architecture
![Alt text](docs/diagrams/FitChaser_Architecture.jpg "Basic Architecture")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this suppose to be a picture?

image

Comment thread docs/DeveloperGuide.md
At app launch, it initializes and loads the components and data in the correct sequence, and connects them up with each other.
At shut down, it shuts down the other components and invokes cleanup methods where necessary.

The bulk of the app’s work is done by the following six components:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should it be in point format instead, its a continuous line now

image

Comment thread docs/DeveloperGuide.md Outdated
and tags are auto-generated:
![Alt text](../docs/diagrams/Sequence Digram for tagging.png "Sequence Diagram for Tagging")
### Sequence Diagram for creating a workout
![Sequence diagram for creating a workout](diagrams/SD_createw.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would you consider using plantUML on IDE to create the diagram, because handwritten diagrams can be hard to read

Comment thread docs/DeveloperGuide.md Outdated
### Sequence Diagram for creating a workout
![Sequence diagram for creating a workout](diagrams/SD_createw.png)
### Sequence Diagram for adding an exercise to current workout
![Sequence diagram for adding an exercise](diagrams/SD_addex.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is the activation bar suppose to continue after return Exercise?

image

Comment thread docs/DeveloperGuide.md Outdated
* Higher computational cost
* Difficult to debug and test

Rationale: ALternative 1 was chosen for simplicity and predictability. For a CLI Fitchaser,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is ALternative a typo?

Comment thread docs/DeveloperGuide.md Outdated
This enables users to quickly identify workout types and track training patterns over time.

### Class Diagram
![Alt text](../docs/diagrams/Class_Diagram_for_tagging_2.png "Class Diagram for Tagging")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this the same issue as above, as the image is not displaying?

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 am getting the same issue, there is no class diagram image showing

Copy link
Copy Markdown

@ry-koh ry-koh left a comment

Choose a reason for hiding this comment

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

Overall, good job on the Developer Guide, it is very in depth, just some minor formatting issues and diagram problems.

Comment thread docs/DeveloperGuide.md Outdated
## UI Component
The API of this component is specified in seedu.fitchasers.ui.UI.

![img.png](img.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For the class diagram, I think there should not be the circle with the C indicating class.
image

Comment thread docs/DeveloperGuide.md Outdated
## UI Component
The API of this component is specified in seedu.fitchasers.ui.UI.

![img.png](img.png)
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 this UML diagram does not follow the convention of our module. There should not be the 'Constants', 'State', 'Ctor', 'Input API', 'Output API', 'Chat Bubble Logic (helpers)' headers.

image

Comment thread docs/DeveloperGuide.md Outdated
## UI Component
The API of this component is specified in seedu.fitchasers.ui.UI.

![img.png](img.png)
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 should also not be the {readOnly} in the constants section. Also, should the constants section be in the class diagram?

image

Comment thread docs/DeveloperGuide.md Outdated
a user's weight and goal weight. It works together with the `Person` entity to maintain a complete
history of weight entries.

![Alt text](docs/diagrams/WeightManager_Class_Diagram.png "Basic Architecture")
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 image does not seem to be loading properly.

image

Comment thread docs/DeveloperGuide.md Outdated

The `GoalWeightTracker` component handles the user's target goal weight. It works together with the `FileHandler` to persist goal data and provides feedback comparing the goal with the user's latest recorded weight.

![Alt text](docs/diagrams/GoalWeightTracker_Class_Diagram.png "Basic Architecture")
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 image does not seem to be loading properly.

image

Comment thread docs/DeveloperGuide.md Outdated
### Sequence Diagram for adding an exercise to current workout
![Sequence diagram for adding an exercise](diagrams/SD_addex.png)
### Sequence Diagram for adding a set to the current exercise
![Sequence diagram for adding a set](diagrams/SD_addset.png)
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 words might be a bit too small after converting to PDF.

image

Comment thread docs/DeveloperGuide.md
* More complex override logic

Rationale: Alternative 1 was chosen as the separation provides clearer semantics and aligns with the use
case where users may want to distinguish between automatic suggestions and their own categorization.
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 is a formatting issue here with an extra indentation.

image

Comment thread docs/DeveloperGuide.md
* Add a `filterByTag(String tag)` method to `WorkoutManager`
* Modify `Workout.getAllTags()` to support efficient tag lookups
#### Proposed feature: Display aggregate statistics grouped by tag
Example command: `/stats --by-tag`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another formatting issue here
image

image

Copy link
Copy Markdown

@Pavithra6-Srinivasan Pavithra6-Srinivasan left a comment

Choose a reason for hiding this comment

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

Most of your diagrams are not showing but other than that its a very detailed DG, good job!

Comment thread docs/DeveloperGuide.md Outdated
#### How the architecture components interact with each other

The Sequence Diagram below shows how the components interact with each other for the scenario where the user issues the command /create_workout pushup.
![Alt text](../docs/diagrams/Architectural_Sequence_Diagram_CW.png "Basic Architecture Sequence Diagram")
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 am not able to see you sequence diagram

Comment thread docs/DeveloperGuide.md Outdated
- Listens to model changes indirectly: Domain managers return updated entities/strings; UI redraws views (e.g., ViewLog.render(...) for monthly logs).


Responsibilities
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would be easier to read if this section was in point form

Comment thread docs/DeveloperGuide.md Outdated

The `WorkoutManager` component is responsible for managing all workout-related operations in FitChasers, including
workout creation, exercise tracking, and workout history management.
![Alt text](docs/diagrams/WorkoutManager_class_diagram.png "Basic Architecture")
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 am not able to see this class diagram

Copy link
Copy Markdown

@Lee-YiSheng Lee-YiSheng left a comment

Choose a reason for hiding this comment

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

overall good sequence diagram in general, easy to understand. As other has commented, may be better to use UML diagram instead of hand written notes

Comment thread docs/DeveloperGuide.md Outdated
This enables users to quickly identify workout types and track training patterns over time.

### Class Diagram
![Alt text](../docs/diagrams/Class_Diagram_for_tagging_2.png "Class Diagram for Tagging")
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 am getting the same issue, there is no class diagram image showing

Comment thread docs/DeveloperGuide.md Outdated
### Sequence Diagram for creating a workout
![Sequence diagram for creating a workout](diagrams/SD_createw.png)
### Sequence Diagram for adding an exercise to current workout
![Sequence diagram for adding an exercise](diagrams/SD_addex.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image Should the workout instance line be angled?

Comment thread docs/DeveloperGuide.md Outdated
### Sequence Diagram for creating a workout
![Sequence diagram for creating a workout](diagrams/SD_createw.png)
### Sequence Diagram for adding an exercise to current workout
![Sequence diagram for adding an exercise](diagrams/SD_addex.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image

Should method returns, showMessage() and displaySuccessMessage be single filled arrows?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is showmessage() following the correct naming convention?
is display success message also following the correct method naming convention?

Comment thread docs/DeveloperGuide.md Outdated
### Sequence Diagram for creating a workout
![Sequence diagram for creating a workout](diagrams/SD_createw.png)
### Sequence Diagram for adding an exercise to current workout
![Sequence diagram for adding an exercise](diagrams/SD_addex.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image Is this the correct self-invocation notation?

bennyy117 and others added 30 commits November 4, 2025 12:29
# Conflicts:
#	src/main/java/seedu/fitchasers/FileHandler.java
#	src/main/java/seedu/fitchasers/ui/ViewLog.java
#	src/main/java/seedu/fitchasers/user/Person.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.