[CS2113-T17-1] AthletiCLI #16

AlWo223 · 2023-10-05T09:20:49Z

AthletiCLI helps athletes track their activities, workouts, diets and well-being in general. It is optimised for CLI users so that they can add their activities very quickly.

Fix Calendar

woowenjun99

The team did a great job in their implementation.

In terms of software engineering, I can see that they abided to most of the good code quality and practices. However, there are 2 things to take note of. Firstly, assertions are different from exceptions. It is good to write many assertions to make the code defensive, but they are just to verify the programmer's assumption that if the line is reached, it should not fail. Error handling still has to be carried out. For example, a proper usage of assertion is :

if (fat < 0) {
// Throw exception here
}
assert fat >= 0;

We have already handled the case that fat is less than 0, and therefore the assertion will definitely pass. This is especially good when there is a chain of responsibility, like if we already handled the exception in method A and put the assertion in method B which depends on the value from method A, it should be true.

Second point would be that I notice many methods in the diet are empty and not implemented yet. It might be better to implement them soon or delete them first since it is just dead code.

In terms of Object Oriented Programming, it is done well in the sense that I can tell there is Single Responsibility Principle and each models have their own attributes. Their activity command class is in charge of causing the various objects to interact with each other

woowenjun99 · 2023-10-24T10:04:33Z

src/test/java/athleticli/ui/ParserTest.java

+
+    @Test
+    void parseDietGoalSetEdit_oneInvalidGoal_throwAthletiException() {
+        String invalidGoalString = "calories/caloreis protein/protein";


I believe this is a constant so you can use capital letters to abide by the code quality, so does the rest of the variables here in this file

woowenjun99 · 2023-10-29T13:11:19Z

docs/DeveloperGuide/AddActivity.png

Perhaps it might be better to use a better variable name instead of a and c?

woowenjun99 · 2023-10-29T13:14:07Z

src/main/java/athleticli/data/activity/ActivityGoalList.java

+    @Override
+    public String unparse(ActivityGoal activityGoal) {
+        // TODO
+        return null;
+    }
+}


Perhaps we can remove these lines of code first since it is unused?

woowenjun99 · 2023-10-29T13:20:22Z

src/main/java/athleticli/ui/Ui.java

+    /**
+     * Constructs a <code>Ui</code> object, whose input <code>in</code>
+     * and output <code>out</code> is the standard input and the standard
+     * output, respectively.
+     */
+    private Ui() {
+        this(System.in, System.out);
+    }


There is no need for function overloading. You can just use the constructor below and pass in the System.in and System.out unless there is a very good reason why you need it

woowenjun99 · 2023-10-29T13:22:59Z

src/main/java/athleticli/ui/Ui.java

+     * @param messages  The messages to be shown.
+     */
+    public void showMessages(String... messages) {
+        assert messages != null : "Messages should not be null";


Do note that assertions are not the same as throwing exceptions. They should not be used to replace the error handling during run time. They are only used to test certain assumptions

woowenjun99 · 2023-10-29T13:24:31Z

src/main/java/athleticli/data/sleep/SleepList.java

+/**
+ * Represents a list of sleep records.
+ */
+public class SleepList extends StorableList<Sleep> implements Findable {


Perhaps we can remove this class because most of the methods are not implemented?

Jonoans

Overall the DG is pretty nice with good amount of details, just the diagrams could be better :D

Jonoans · 2023-11-01T04:37:12Z

docs/DeveloperGuide.md

+This following sequence diagram show how the 'set-diet-goal' command works:
+
+<p  align="center" width="100%">
+  <img width="80%" src="images/setDietGoalUmlSequenceDiagram.svg" alt="'set-diet-goal' Sequence Diagram"/>


Maybe the diagram needs to be updated to reflect the current implementation? It seems to be a little different.

Jonoans · 2023-11-01T04:38:10Z

docs/DeveloperGuide.md

+This following sequence diagram show how the 'set-diet-goal' command works:
+
+<p  align="center" width="100%">
+  <img width="80%" src="images/setDietGoalUmlSequenceDiagram.svg" alt="'set-diet-goal' Sequence Diagram"/>


Maybe it would help to include variable names in the diagram? I.e. it may not be immediately clear which object's execute() is called unless we look into the code/make educated guesses

Jonoans · 2023-11-01T04:41:06Z

docs/DeveloperGuide.md

-|--------|----------|---------------|------------------|
-|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 ...                                                                      |


Nice list of stories!

WooKaiNing

Overall DG is detailed, good use of sequence diagrams

WooKaiNing · 2023-11-01T04:18:19Z

docs/DeveloperGuide.md

+
+**Step 5 - User Interaction:** Once the activity is successfully added, a confirmation message is displayed to the user.
+
+The following sequence diagram shows how the `add-activity` operation works:


may want to maintain same format of explaining feature implementation to ensure consistency; ie. similar to [Implemented] Setting Up of Diet Goals where image of sequence diagram is first attached before detailed explanation.

WooKaiNing · 2023-11-01T04:29:31Z

docs/DeveloperGuide.md

+| v1.0    | sleep deprived user             | delete a sleep entry       | remove outdated or incorrect data from my sleep records                                |
+| v1.0    | sleep deprived user             | view all my sleep records  | have a clear overview of my sleep habits and make informed decisions on my sleep       |
+| v1.0    | sleep deprived user             | edit my sleep entries      | correct any mistakes or update my sleep information as needed                          |
+| v2.0    | user                            | find a to-do item by name  | locate a to-do without having to go through the entire list                            |


may want to be more specific in identifying what are the possible criterions of "to-do". for example, is it referring to an activity / diet / sleep entry, or is it the goals?

WooKaiNing · 2023-11-01T04:36:51Z

docs/DeveloperGuide.md

+Step 8. After executing the SetDietGoalCommand, SetDietGoalCommand returns a message that is passed to 
+AthletiCLI to be passed to UI(not shown) for display.
+
+#### [Implemented] Adding activities


might be better to put the adding of activities in a separate section instead of under diet goals since the adding of activities is not related to the diet goals based on sequence diagram.

WooKaiNing · 2023-11-01T04:45:14Z

docs/DeveloperGuide.md

+
+### Sleep Management in AthletiCLI
+
+#### [Implemented] Adding, Editing, Deleting, Listing Sleep


is there find function for sleep entries as well?

WooKaiNing · 2023-11-01T05:38:22Z

docs/DeveloperGuide.md

+
+This following sequence diagram show how the 'set-diet-goal' command works:
+
+<p  align="center" width="100%">


include self-invocation of initializeIntermmediateDietGoals method?

lckjosh

Overall a good and comprehensive DG, just a few tiny nits!

lckjosh · 2023-11-02T09:12:16Z

docs/images/AddActivity.svg

The black words ":AthletiCLI" is a bit hard to read against the dark background. Perhaps a different colour would make it more readable.

lckjosh · 2023-11-02T09:12:58Z

docs/images/HelpAddDiet.svg

Perhaps a consistent use of colour for the different entities across sequence diagrams would make things look a little neater.

lckjosh · 2023-11-02T09:17:16Z

docs/DeveloperGuide.md

-|--------|----------|---------------|------------------|
-|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 ...                                                                      |


Perhaps a space between 'Version' and 'As a...' would make things more readable.

lckjosh · 2023-11-02T09:18:51Z

docs/images/DataClassDiagram.svg

Perhaps the arrow from ActivityList to StorableList could be straight instead of curved, and also not overlap with the arrow from DietList to StorableList.

lckjosh · 2023-11-02T09:24:33Z

docs/images/SleepAndSleepListClassDiagram.svg

Perhaps the lines could be straight and not curved to make it look slightly neater.

lckjosh · 2023-11-02T09:28:15Z

docs/images/setDietGoalUmlSequenceDiagram.svg

Perhaps you could edit the diagram such that the lifeline stops after the X to conform to the UML standard we learnt in class.

ken-ruster · 2023-11-02T09:23:59Z

docs/DeveloperGuide.md

+
+### Overview
+
+The class diagram shows the relationship between `AthletiCLI`, `Ui`, `Parser`, and `Data`.


Should your class diagram show the accessibility of the functions in it?

Thanks for the feedback! However we cannot seem to find a way to label it with PUML. Perhaps you could share the software that you are using for your class diagrams.

Your UML Diagramm looks good to me but it might be better to delete the square used for attributes and methods if this class doesn’t have it at all.

ken-ruster · 2023-11-02T09:27:28Z

docs/DeveloperGuide.md

+
+The _Sequence Diagram_ below shows how the components interact with each other for the scenario where the user issues the command `help add-diet`.
+
+![](images/HelpAddDiet.svg)


Well-made diagram!

ken-ruster · 2023-11-02T09:28:30Z

docs/DeveloperGuide.md

+
+The `Storage` component only interacts with the `Data` component. The _Sequence Diagram_ below shows how they interact with each other for the scenario where a `save` command is executed.
+
+![](images/Save.svg)


Data is not deactivated after return arrow. Should this be the case?

ken-ruster · 2023-11-02T09:30:36Z

docs/DeveloperGuide.md

+    - `list-diet` for listing all diets.
+    - `find-diet 2021-09-01` for finding diets of a particular date.
+
+2. **Command Identification**: The Parser Class identifies the type of diet operation and passes the necessary parameters.


In published website, the numbering reads 1., 7, ..., 10. Is there something that can be done about this?

ken-ruster · 2023-11-02T09:35:00Z

docs/DeveloperGuide.md

+1. **Step 1 - Input Capture:** The user issues a `set-activity-goal ...` which is captured and passed to the 
+   Parser by the running AthletiCLI instance.
+2. **Step 2 - Goal Parsing:** The Parser parses the raw input to obtain the sports, target and timespan of the goal. 
+   Given that all these parameters are provided correctly and no exception is thrown, a new activity goal object is 
+   created.
+3. **Step 3 - Command Parsing:** In addition the parser will create a `SetActivityGoalCommand` object with the newly 
+   added activity goal attached to it. The command implements the `SetActivityGoalCommand#execute()` operation and is 
+   passed to the AthletiCLI instance.
+4. **Step 4 - Goal Addition:** The AthletiCLI instance executes the `SetActivityGoalCommand` object. The command will 
+   access the data and retrieve the currently stored list of activity goals stored inside it. The new `ActivityGoal` 
+   object is added to the list.


Good format! Labelling each of the steps makes it easy to understand what is being done in each step

ken-ruster · 2023-11-02T09:37:45Z

docs/DeveloperGuide.md

+<p  align="center" >
+  <img width="100%" src="images/AddActivityGoal.svg" alt="Sequence Diagram of set-activity-goal"/>
+</p>
+


The colour for the AtheleticCLI class background makes it difficult to read on some screens. Consider changing it to a color with more contrast to the black text?

ken-ruster · 2023-11-02T09:39:41Z

docs/DeveloperGuide.md

+<p  align="center" width="100%">
+  <img width="80%" src="images/SleepAndSleepListClassDiagram.svg" alt="Class Diagram of Sleep and SleepList"/>


Class diagram is extremely cluttered with methods. Are there any methods which are less important to the overall functionality which can be removed? Doing so might make it easier to read and understand.

charkty

Overall, I like the variety of diagrams and the detail that your document guide has. Some diagrams could be simplified (adding only relevant methods) but overall, it is a very good DG!

charkty · 2023-11-03T03:24:26Z

docs/images/MainClassDiagram.svg

Should your arrows be pointing upwards instead?

charkty · 2023-11-03T03:25:41Z

docs/images/Save.svg

I like that you started from SaveCommand for simplicity.

charkty · 2023-11-03T03:31:13Z

docs/images/SleepAndSleepListClassDiagram.svg

I like the variety of this class diagram where inheritance and dependencies are used. Can dependency be mixed with inheritance for your arrow from SleepList to Findable?

charkty · 2023-11-03T03:36:25Z

docs/images/setDietGoalUmlSequenceDiagram.svg

Should there be a broken activation bar?

charkty · 2023-11-03T03:40:41Z

docs/DeveloperGuide.md

+**Step 1:** The input from the user ("set-diet-goal fats/1") runs through AthletiCLI to the Parser Class.
+
+**Step 2:** The Parser Class will identify the request as setting up a diet goal and pass in the parameters
+"fats/1".
+
+**Step 3:** A temporary dietGoalList is created to store newly created diet goals.

-{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
+**Step 4:** The inputs are verified against our lists of approved diet goals.
+
+**Step 5:** For each of the diet goals that are valid, a dietGoal object will be created and stored in the 
+temporary dietGoalList.
+
+**Step 6:** The Parser then creates for an instance of SetDietGoalCommand and returns the instance to 
+AthletiCLI.
+
+**Step 7:** AthletiCLI will execute the SetDietGoalCommand. This adds the dietGoals that are present in the 
+temporary list into the data instance of DietGoalList which will be kept for records.
+
+**Step 8:** After executing the SetDietGoalCommand, SetDietGoalCommand returns a message that is passed to 
+AthletiCLI to be passed to UI(not shown) for display.


I like how clear your steps are and how straight to the point it is.

charkty · 2023-11-03T03:45:53Z

docs/DeveloperGuide.md

+The fulfillment of these goals is tracked automatically and can be evaluated by the user at any time.
+
+These are the key components and their roles in the architecture of the goal tracking:
+* `SetActivityGoalCommand`: encapsulates the execution of the `set-activity-goal` command. It adds 


is the set-activity-goal command a bit long for a cli input?

Haoyuli2002

Your DG is quite good but some modification or improvement might make it even better!

Haoyuli2002 · 2023-11-03T03:32:46Z

docs/puml/HelpAddDiet.puml

+AthletiCLI -> Ui++ : showMessages(feedback)
+Ui --> AthletiCLI--
+
+destroy HelpCommand


Overall I think this is a pretty good sequence diagram, however, I think it might be better to separate the cross sign and the dash line of your method, you may see the marks in the following screenshot

Haoyuli2002 · 2023-11-03T03:40:55Z

docs/DeveloperGuide.md

+
+### Overview
+
+The class diagram shows the relationship between `AthletiCLI`, `Ui`, `Parser`, and `Data`.


Your UML Diagramm looks good to me but it might be better to delete the square used for attributes and methods if this class doesn’t have it at all.

Haoyuli2002 · 2023-11-03T03:49:19Z

docs/DeveloperGuide.md

+
+By following these general steps, AthletiCLI ensures a streamlined process for managing diet-related tasks.
+
+#### [Implemented] Setting Up of Diet Goals


This diagram looks generally comprehensive but I think you the activation bar should end only if the self invocation is done.

Also I think it might be better that the cross signal indicating the end of the activation bar should be attached directly to the activation bar.

Improve test coverage for diet goals

User Guide Update and Numerous Bug Fix

Co-authored-by: Yang Ming-Tian <[email protected]>

Update skylee03's PPP

Update README.md

Add captions for DG images for PPP

Add sleep command and sleep list feature to Developers Guide

Fix DG numbering

Final minor bugfixes for sleep

Minor update to UG

Replace 'command' with `command` in DG

PPP Update

PPP Update - Fix Page Layout

Brian030601 added a commit to Brian030601/tp that referenced this pull request Oct 18, 2023

Merge pull request nus-cs2113-AY2324S1#16 from Brian030601/master

85ff53a

Fix Calendar

woowenjun99 reviewed Oct 29, 2023

View reviewed changes

Jonoans reviewed Nov 1, 2023

View reviewed changes

WooKaiNing reviewed Nov 1, 2023

View reviewed changes

lckjosh reviewed Nov 2, 2023

View reviewed changes

ken-ruster reviewed Nov 2, 2023

View reviewed changes

charkty reviewed Nov 3, 2023

View reviewed changes

Haoyuli2002 reviewed Nov 3, 2023

View reviewed changes

yicheng-toh and others added 22 commits November 12, 2023 21:51

Merge pull request #284 from yicheng-toh/v2.1_dietgoals

fb3eea5

Improve test coverage for diet goals

Implement parseNonNegativeInteger function

e36a733

Remove repetitive functions

d866750

Do not allow seconds to be passed in datetime

3f25007

Update message MESSAGE_DATE_FUTURE

ee3c7e5

Merge upstream/master into userguide-update

6cb8b0c

Remove redundant empty new line

a22144b

Update the help messages for activity goal

fb20f3a

Update the text-ui-test EXPECTED.TXT

8cf94ee

Merge master in DG branch

c533937

Update the phrasing in UG

82c6ae2

Update activity edit and add swim command format in UG summary and help

ba378de

Update skylee03's PPP

d1848d2

Merge pull request #288 from nihalzp/userguide-update

eb84885

User Guide Update and Numerous Bug Fix

Fix variable naming of elevationBoundary

4389282

Merge remote-tracking branch 'upstream/master' into ppp

8751a32

Apply suggestions from code review

dcb1906

Co-authored-by: Yang Ming-Tian <[email protected]>

Merge pull request #302 from skylee03/ppp

d16e354

Update skylee03's PPP

Remove sleep attack test cases

c901657

Fix JavaDoc coding standard violations

f584322

Squash diet goal bugs

4609f15

Refactoring of diet goal list methods

b37e982

DaDevChia and others added 30 commits November 14, 2023 01:52

Added spacing to DG

f02ee1f

Merge pull request #358 from skylee03/docs

87d54d2

Update README.md

Merge branch 'master' into branch-UserAndDeveloperGuide

54ca2b1

Add captions for DG images for PPP

523c84f

Add contribution to PPP

2aa66fc

Merge pull request #360 from yicheng-toh/docs

14ac18c

Add captions for DG images for PPP

Update DeveloperGuide.md with Miscellaneous

4dfec50

Refactor Dylan Chia's portfolio page

70d5d97

Restored Miscellaneous in DG

dfc2143

Updated PPP for dylan

29f2d18

Merge pull request #359 from DaDevChia/branch-UserAndDeveloperGuide

045f747

Add sleep command and sleep list feature to Developers Guide

Fix DG numbering

2abdd09

Update sleep record index message

ff17050

Update find-sleep command syntax

17e3f47

Update UserGuide for lack of Goal Delete for

999fc8b

Updated text-ui-test

7b36025

Merge pull request #361 from skylee03/docs

44af977

Fix DG numbering

Merge pull request #362 from DaDevChia:branch-UserAndDeveloperGuide

22c9fea

Final minor bugfixes for sleep

Minor update to UG

9445c93

Merge pull request #363 from DaDevChia/branch-UserAndDeveloperGuide

09f33aa

Minor update to UG

Replace 'command' with command in DG

e693e34

Merge pull request #364 from skylee03/docs

12cf8bb

Replace 'command' with `command` in DG

Update PPP

1b6e4dd

Fix typo in ppp

80d117d

Fix typo in ppp

d6b8b6f

Fix typo in PPP features

bb03eb4

Merge pull request #365 from nihalzp/ppp-update

6bb9fa7

PPP Update

Fix page layout

13ec792

Fix page layout typo

e595239

Merge pull request #366 from nihalzp/ppp-update

7ab597b

PPP Update - Fix Page Layout


		Step 5 - User Interaction: Once the activity is successfully added, a confirmation message is displayed to the user.

		The following sequence diagram shows how the `add-activity` operation works:


		### Sleep Management in AthletiCLI

		#### [Implemented] Adding, Editing, Deleting, Listing Sleep


		This following sequence diagram show how the 'set-diet-goal' command works:

		<p align="center" width="100%">


		### Overview

		The class diagram shows the relationship between `AthletiCLI`, `Ui`, `Parser`, and `Data`.


		The _Sequence Diagram_ below shows how the components interact with each other for the scenario where the user issues the command `help add-diet`.

		![](images/HelpAddDiet.svg)


		The `Storage` component only interacts with the `Data` component. The _Sequence Diagram_ below shows how they interact with each other for the scenario where a `save` command is executed.

		![](images/Save.svg)

		<p align="center" width="100%">
		<img width="80%" src="images/SleepAndSleepListClassDiagram.svg" alt="Class Diagram of Sleep and SleepList"/>


		By following these general steps, AthletiCLI ensures a streamlined process for managing diet-related tasks.

		#### [Implemented] Setting Up of Diet Goals

[CS2113-T17-1] AthletiCLI #16

Are you sure you want to change the base?

[CS2113-T17-1] AthletiCLI #16

Conversation

AlWo223 commented Oct 5, 2023 • edited Loading

woowenjun99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jonoans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WooKaiNing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WooKaiNing Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lckjosh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charkty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Haoyuli2002 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlWo223 commented Oct 5, 2023 •

edited

Loading

WooKaiNing Nov 1, 2023 •

edited

Loading