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-W13-2] FitTrack #22

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

Conversation

TheDinos
Copy link

FitTrack is a desktop application designed to assist students in training for the National Physical Fitness Award (NAPFA) test. It focuses on streamlining data entry through a Command Line Interface (CLI) for students who seek efficiency and focus.

}
}
return 0; // return 0 points as performance is below the minimum standard
} catch (InvalidAgeException e) {
Copy link

Choose a reason for hiding this comment

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

Is the variable name "e" informative enough?

// Utility method to add age sub-table for a specific gender
protected static void addAgeSubTable(Map<LookUpKey, TreeMap<Integer, Integer>> pointsTable,
Gender gender, int age, int[][] points, boolean reverseOrder) {
assert age >= 12 && age <= 24 : "Age should be within 12 and 24 during table initialisation"; //
Copy link

Choose a reason for hiding this comment

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

Accidental empty comment?


// Until the exit command is entered, execute command then read user input
while (!input.equals(EXIT_COMMAND)) {
assert !input.trim().isEmpty() : "User input should not be null or empty";
Copy link

Choose a reason for hiding this comment

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

Is this supposed to be an assertion or exception?

TreeMap<Integer, Integer> ageSubTable;
if (reverseOrder){
ageSubTable = new TreeMap<>(Comparator.reverseOrder());
} else{
Copy link

Choose a reason for hiding this comment

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

Java reserved words should be followed by a white space

Comment on lines 11 to 12
private static final Map<LookUpKey, TreeMap<Integer, Integer>> pullUpTable = new HashMap<>();
private static final boolean reverseOrder = true;
Copy link

Choose a reason for hiding this comment

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

Constant names must be all uppercase using underscore to separate words

Comment on lines +30 to +33
@Override
public String getName() {
return name;
}
Copy link

Choose a reason for hiding this comment

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

If implementation of getName() is the same for every child class, you may consider implementing it in the parent class instead

Comment on lines 19 to 20
public static final String SAVE_FILE = "data/saveFile.txt"; // Path to the save file
public static final File SAVEFILE = new File(SAVE_FILE); // File object for the save file
Copy link

Choose a reason for hiding this comment

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

Consider using more meaningful variable names, SAVE_FILE and SAVEFILE does not provide much context as to what differentiates them, leading to confusion when using it in the code

// Assert that the session list is not null before loading
assert sessionList != null : "Session list must not be null";

Scanner s = new Scanner(SAVEFILE); // Create a Scanner to read the save file
Copy link

Choose a reason for hiding this comment

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

Use more informative variable names

Comment on lines 30 to 87
public static void parse(User user, String input, ArrayList<TrainingSession> sessionList) {
assert input != null : "Input must not be null";
assert user != null : "User object must not be null";
assert sessionList != null : "Session list must not be null";

String[] sentence = {input, input};
String command = input;
String description = "";

// Split the input into command and description if applicable
if (input.contains(" ")) {
sentence = input.split(" ", 2);
command = sentence[0];
description = sentence[1];
}

switch (command) {
case SET_USER_COMMAND:
assert description.contains(" ") : "Invalid user data format";
sentence = description.split(" ", 2);
user.setGender(sentence[0]);
user.setAge(sentence[1]);
break;
case ADD_SESSION_COMMAND:
assert !description.isEmpty() : "Session description must not be empty";
LocalDateTime currentTime = LocalDateTime.now();
sessionList.add(new TrainingSession(currentTime, description, user));
printAddedSession(sessionList);
break;
case EDIT_EXERCISE_COMMAND:
sentence = description.split(" ", 3);
assert sentence.length == 3 : "Edit exercise command requires exactly 3 arguments";
int sessionIndex = Integer.parseInt(sentence[0]) - 1;
int exerciseIndex = Integer.parseInt(sentence[1]);
String exerciseData = sentence[2];
assert sessionIndex >= 0 && sessionIndex < sessionList.size() : "Session index out of bounds";
sessionList.get(sessionIndex).editExercise(exerciseIndex, exerciseData);
break;
case LIST_SESSIONS_COMMAND:
printSessionList(sessionList); // Print the list of sessions
break;
case VIEW_SESSION_COMMAND:
int viewIndex = Integer.parseInt(description) - 1;
assert viewIndex >= 0 && viewIndex < sessionList.size() : "View session index out of bounds";
printSessionView(sessionList, viewIndex); // Print the session view
break;
case DELETE_SESSION_COMMAND:
int indexToDelete = Integer.parseInt(description) - 1;
assert indexToDelete >= 0 && indexToDelete < sessionList.size() : "Delete session index out of bounds";
TrainingSession sessionToDelete = sessionList.get(indexToDelete);
sessionList.remove(indexToDelete);
printDeletedSession(sessionList, sessionToDelete);
break;
default:
printUnrecognizedInputMessage(); // Response to unrecognized inputs
break;
}
}
Copy link

Choose a reason for hiding this comment

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

Try to avoid long methods. Perhaps consider extracting out the commands into separate methods

Comment on lines 90 to 237
System.out.println(e.getMessage());
}
printSessionView(sessionList, sessionIndex);
break;
case LIST_SESSIONS_COMMAND:
printSessionList(sessionList); // Print the list of sessions
break;
case VIEW_SESSION_COMMAND:
int viewIndex = Integer.parseInt(description) - 1;
assert viewIndex >= 0 && viewIndex < sessionList.size() : "View session index out of bounds";
printSessionView(sessionList, viewIndex); // Print the session view
break;
case DELETE_SESSION_COMMAND:
int indexToDelete = Integer.parseInt(description) - 1;
assert indexToDelete >= 0 && indexToDelete < sessionList.size() : "Delete session index out of bounds";
TrainingSession sessionToDelete = sessionList.get(indexToDelete);
sessionList.remove(indexToDelete);
printDeletedSession(sessionList, sessionToDelete);
break;
case ADD_REMINDER_COMMAND:
sentence = description.split(" ", 2);

String inputDeadline = sentence[1];
description = sentence[0];

assert !description.isEmpty() : "Reminder description must not be empty";
assert !Objects.equals(inputDeadline, "") : "Reminder deadline must not be empty";
LocalDateTime deadline = parseReminderDeadline(inputDeadline);
reminderList.add(new Reminder(description, deadline, user));
printAddedReminder(reminderList);
break;
case DELETE_REMINDER_COMMAND:
int reminderIndexToDelete = Integer.parseInt(description) - 1;
assert reminderIndexToDelete >= 0 && reminderIndexToDelete < reminderList.size() : "Delete reminder index "
+ "out of bounds";
Reminder reminderToDelete = reminderList.get(reminderIndexToDelete);
reminderList.remove(reminderIndexToDelete);
printDeletedReminder(reminderList, reminderToDelete);
break;
case LIST_REMINDER_COMMAND:
printReminderList(reminderList);
break;
case LIST_UPCOMING_REMINDER_COMMAND:
beginSegment();
printUpcomingReminders(reminderList);
break;

case "add-goal": // use "add-goal" consistently in input and command handling
if (!description.isEmpty()) {
String[] goalParts = description.split(" ", 2);
String goalDescription = goalParts[0];
LocalDateTime goalDeadline = null;

if (goalParts.length > 1) {
String goalDeadlineInput = goalParts[1];
try {
goalDeadline = parseGoalDeadline(goalDeadlineInput);
} catch (IllegalArgumentException e) {
System.out.println("Invalid date format: " + e.getMessage());
return;
}
}
Goal newGoal = new Goal(goalDescription, goalDeadline);
goalList.add(newGoal);
printAddedGoal(goalList);
} else {
System.out.println("Please specify a goal to add.");
}
break;

case "delete-goal":
try {
int index = Integer.parseInt(description) - 1;
if (index >= 0 && index < goalList.size()) {
goalList.remove(index);
System.out.println("Goal at index " + (index + 1) + " has been removed.");
} else {
System.out.println("Invalid goal index.");
}
} catch (NumberFormatException e) {
System.out.println("Please specify a valid index to delete.");
}
break;

case "list-goal":
if (goalList.isEmpty()) {
System.out.println("No goals to display.");
} else {
System.out.println("Goals:");
for (int i = 0; i < goalList.size(); i++) {
System.out.println((i + 1) + ". " + goalList.get(i));
}
}
break;

default:
printUnrecognizedInputMessage(); // Response to unrecognized inputs
break;
}
}
Copy link

Choose a reason for hiding this comment

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

Method is too long. Do consider extracting out code for each command to a separate method

*
* @param inputDeadline A string input by the user. Intended format is DD/MM/YYYY or DD/MM/YYYY HH:mm:ss.
* @return A {@code LocalDateTime} object indicating reminder deadline
* @throws IllegalArgumentException
Copy link

Choose a reason for hiding this comment

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

Missing description for exception

// Try to parse with time (dd/MM/yyyy HH:mm:ss)
try {
return LocalDateTime.parse(inputDeadline, dateTimeFormatter);
} catch (DateTimeParseException e) {
Copy link

Choose a reason for hiding this comment

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

Use more descriptive variable names, it is unclear what "e" refers to


public void printReminderDescription() {
System.out.print(this.reminderDescription + " | " +
this.reminderDeadline.format(DateTimeFormatter.ofPattern("dd/MM/yyyy HH:mm")) + System.lineSeparator());
Copy link

Choose a reason for hiding this comment

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

Line length should be no longer than 120 chars


public void addGoal(User user) {
user.addGoal(goal); // Add the goal object to the user's goal list
// Use the correct method to get the description of the goal
Copy link

Choose a reason for hiding this comment

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

Comments should explain the WHAT and WHY aspects of the code, rather than the HOW aspect. In this case, a comment may not be necessary

private final String description;
private final LocalDateTime deadline;

// Constructor
Copy link

Choose a reason for hiding this comment

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

Would be good to follow Javadoc format to document methods instead

return deadline;
}

// Optionally, you might want to override the toString() method for easy printing
Copy link

Choose a reason for hiding this comment

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

Was this comment intended?

Comment on lines 185 to 206
case "add-goal": // use "add-goal" consistently in input and command handling
if (!description.isEmpty()) {
String[] goalParts = description.split(" ", 2);
String goalDescription = goalParts[0];
LocalDateTime goalDeadline = null;

if (goalParts.length > 1) {
String goalDeadlineInput = goalParts[1];
try {
goalDeadline = parseGoalDeadline(goalDeadlineInput);
} catch (IllegalArgumentException e) {
System.out.println("Invalid date format: " + e.getMessage());
return;
}
}
Goal newGoal = new Goal(goalDescription, goalDeadline);
goalList.add(newGoal);
printAddedGoal(goalList);
} else {
System.out.println("Please specify a goal to add.");
}
break;
Copy link

Choose a reason for hiding this comment

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

Avoid deep nesting

String inputDeadline = sentence[1];
description = sentence[0];

assert !description.isEmpty() : "Reminder description must not be empty";
Copy link

Choose a reason for hiding this comment

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

Is this intended to be an assertion or exception?

@@ -6,8 +6,77 @@

## Design & implementation

# High Level Functionalities
![HighLevel.png](HighLevel.png)

Choose a reason for hiding this comment

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

Are the arrows standard?
image

@@ -6,8 +6,77 @@

## Design & implementation

# High Level Functionalities
![HighLevel.png](HighLevel.png)

Choose a reason for hiding this comment

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

Could the roles be closer to the arrow?


The following sequence diagram illustrates the function calls involved in this process:

![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)

Choose a reason for hiding this comment

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

Stickman is not standard here
image


The following sequence diagram illustrates the function calls involved in this process:

![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)

Choose a reason for hiding this comment

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

Could activation bars be added?
image


3. **Calculator Logic**: The calculator class uses a lookup table, which maps the user's performance
to points based on their age and gender. The points are returned to the exercise station, where they are stored.


## Product scope

Choose a reason for hiding this comment

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

image
Could add own user stories

Additionally, the state diagram below shows the end state of the `editExercise` function after execution of the command,
`editExercise(Exercise.PULL_UP, 1)`:

![TrainingSessionEditState.png](TrainingSessionEditState.png)

Choose a reason for hiding this comment

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

image
This is not a state diagram

Copy link

@Progresst-8 Progresst-8 left a comment

Choose a reason for hiding this comment

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

DG looks mostly okay, but some amendments are required.
Some parts of the DG are still left unchanged from the template. Those should be updated to reflect your project.

Choose a reason for hiding this comment

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

What is the relation between ExerciseStation class and the 6 station classes below? Is there some kind of inheritance?

The class diagram has non-standard notation such as:
visibility image
class label image
arrowhead Screenshot 2024-10-30 121253


![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)

Additionally, the state diagram below shows the end state of the `editExercise` function after execution of the command,

Choose a reason for hiding this comment

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

The state diagram is a specific kind of UML behavioral diagram.

Additionally, the state diagram below ...

I don't think that's what you're referring to here, as this seems like structural?
Additionally, is this a class diagram or an object diagram? It contains the non-underlined name (like a class diagram) and values of variables (like an object diagram)

Choose a reason for hiding this comment

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

Why is the Parser class represented by a stickfigure? Doesn't the stickfigure represent a user, not a class?

The instance name is missing the colon before it, as per Sequence diagram conventions.

Why are the activation bars missing?

Does Parser have a custom-defined print method, since print() is pointing to Parser? If so, why is a Parser printing (writing to output), when it should be parsing (reading from input)?


3. **Calculator Logic**: The calculator class uses a lookup table, which maps the user's performance
to points based on their age and gender. The points are returned to the exercise station, where they are stored.

Choose a reason for hiding this comment

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

Are the user stories below still unchanged from the template?
Where are the user stories for your project?

Choose a reason for hiding this comment

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

Is this supposed to be a high-level overview of your entire program? Inclusion of low-level implementation details seems counter to what an overview is supposed to do.
If so, isn't a class diagram not appropriate for this purpose?
Perhaps this should be an architecture diagram instead of a class diagram?

Additionally, for a class diagram, labels require the arrowhead, which is not present here.
image
image

initializes an EnumMap, which populates the various `ExerciseStation` instances with their initial values.
Below is a representation of how the `ExerciseStations` are initialized:

![TrainingSessionInitialState.png](TrainingSessionInitialState.png)
Copy link

Choose a reason for hiding this comment

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

Unclear what the visibility is in this class diagram
image

@@ -6,8 +6,77 @@

## Design & implementation

# High Level Functionalities
![HighLevel.png](HighLevel.png)
Copy link

Choose a reason for hiding this comment

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

Somewhat complex class diagram. Maybe split into components?


The following sequence diagram illustrates the function calls involved in this process:

![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

I believe doesn't follow correct format for class name. I think it should be ":ClassName"
image


The following sequence diagram illustrates the function calls involved in this process:

![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Should Parser be shown as an object? i.e. should it be in a box?
image

method. The main responsibility of this method is to invoke the `calculatePoints()` function from the
respective **calculator** class (e.g., `PullUpCalculator`, `SitUpCalculator`), which holds the points calculation logic.

![getPointsSequenceDiagram.png](getPointsSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Should sequence diagram return arrows be labeled "return {variable}" or just "variable"?
image

method. The main responsibility of this method is to invoke the `calculatePoints()` function from the
respective **calculator** class (e.g., `PullUpCalculator`, `SitUpCalculator`), which holds the points calculation logic.

![getPointsSequenceDiagram.png](getPointsSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Could the "PullUpStation" and "PullUpCalculator" be named to be generic names to showcase what is written in the description?


The following sequence diagram illustrates the function calls involved in this process:

![editExerciseSequenceDiagram.png](editExerciseSequenceDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Should activation bars be either added here or removed in the next sequence diagram to follow a standard format?

Choose a reason for hiding this comment

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

This diagram is abit hard to look at
image

Choose a reason for hiding this comment

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

The relationship label can be placed uniformly for better layout
Uploading image.png…

Choose a reason for hiding this comment

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

the use of the uses label on all associations might be redundant, as it doesn’t add much information and could be removed for a cleaner look
Uploading image.png…

Choose a reason for hiding this comment

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

If Calculator is intended to be a shared utility, can the relationship between Calculator and ExerciseStation changed to a dependency that is accessed via FitTrack or TrainingSession instead?
image

Choose a reason for hiding this comment

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

Overall, really well structured DG, diagrams really clarify the functionalities of the app

Zackermax and others added 20 commits November 7, 2024 14:18
…into 205-docs-update-dg-for-v20

# Conflicts:
#	docs/DeveloperGuide.md
# Conflicts:
#	FitTrackLogger.log
…into 205-docs-update-dg-for-high-level-functionalities
…el-functionalities

205 docs update dg for high level functionalities
…aphing-functionalities

233 implement user input for graphing functionalities
- include functionality of Calculator class
- include PUML diagram for graphPerformanceTime
TheDinos and others added 30 commits November 12, 2024 11:34
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.

10 participants