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-F15-2] LifeTrack #56

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

Conversation

paturikarthik
Copy link

LifeTrack is a desktop app for students to track their health data, optimized for use via a Command Line Interface (CLI). LifeTrack is especially catered towards those who can type fast, as they can track their health data much faster than when using traditional Graphical User Interface (GUI) apps.

@paturikarthik paturikarthik reopened this Mar 14, 2024
JeffinsonDarmawan pushed a commit to JeffinsonDarmawan/tp that referenced this pull request Mar 27, 2024
yeozongyao pushed a commit to yeozongyao/tp that referenced this pull request Apr 1, 2024

The Class diagram for Calories list feature is shown below. Unrelated attributes and Classes were excluded.

![CaloriesListClassDiagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/rexyyong/tp/DevGuideRex/docs/CaloriesListClassDiagram.puml)

Choose a reason for hiding this comment

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

Could use these proper symbols for representing public/private classes instead of squares

  • : public
  • : private

: protected

~ : package private


The Class diagram for Calories delete feature is shown below:

![CaloriesDeleteClassDiagram](https://github.com/a-wild-chocolate/tp/blob/master/docs/caloriesDeleteUML.jpg)

Choose a reason for hiding this comment

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

Diagram has formatting issues and does not show up as a picture

Choose a reason for hiding this comment

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

add a end of lifeline "X" after entry deleted

Choose a reason for hiding this comment

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

Change the # to :
Should be Ui:handleUserInput()

- Step 5: The latest hydration list will be updated to saving file by calling `HydrationList#updateFile()`.

The Sequence diagram for Hydration delete feature is shown below:
![HydrationDeleteDiagram.png](HydrationDeleteDiagram.png)

## Product scope
### Target user profile

Choose a reason for hiding this comment

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

Could fill in information on scope, user profile etc

* @return an Entry object representing calorie intake
* @throws InvalidInputException if the input string is missing components or contains empty fields
*/
public static Entry parseCaloriesInput(String input) throws InvalidInputException {

Choose a reason for hiding this comment

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

Perhaps can refactor this method more? It seems too wordy.

Choose a reason for hiding this comment

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

After the updateFile(), from the code it seems like it calls fileHandler in the storage. Perhaps can show the storage class too?

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" beside your classes?

Choose a reason for hiding this comment

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

Are the symbols used to represent "private" and "public" methods correct?

Choose a reason for hiding this comment

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

Perhaps can standardise the class diagrams? Some diagrams have colour some doesn't.


The Class diagram for Calories delete feature is shown below:

![CaloriesDeleteClassDiagram](https://github.com/a-wild-chocolate/tp/blob/master/docs/caloriesDeleteUML.jpg)

Choose a reason for hiding this comment

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

This diagram does not show. It doesn't seem to appear in the docs package as well.

Choose a reason for hiding this comment

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

This sequence diagram looks different from the previous diagram. Perhaps can standardise it?

Copy link

@itsmejr257 itsmejr257 left a comment

Choose a reason for hiding this comment

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

Some cosmetic changes regarding diagrams used requested. Overall, great job!


The sequence diagram for this feature is shown below:

![CaloriesAddEntrySeqDiagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/rexyyong/tp/RexDG/docs/CaloriesAddEntrySeqDiagram.puml)

Choose a reason for hiding this comment

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

There appears to be some self-called methods in ParserCalories#parseCaloriesInput() such as checkKeyWordExists(), checkKeywordsCorrectlyOrdered() etc. Would it be better to include these self called methods into the diagram?

Comment on lines 58 to 66
#### Design considerations

- **Alternative 1 (current choice):** Uses an API to get the calories needed
- Pros: No need to figure out the optimal algorithm
- Cons: Need to parse response to sieve out necessary information

- **Alternative 2:** Uses an algorithm to find the number of calories needed
- Pros: Not dependent on external APIs
- Cons: Need to come up with an algorithm to use

Choose a reason for hiding this comment

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

Great indication of other design considerations. The implementation for this feature is clearly stated and easy to follow. Would it be better to also include a sequence diagram to trace the code?


The Class diagram and sequence diagram for Calories list feature is shown below. Unrelated attributes and Classes were excluded.

![CaloriesListClassDiagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/rexyyong/tp/DevGuideRex/docs/CaloriesListClassDiagram.puml)

Choose a reason for hiding this comment

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

Good job on using a Class Diagram here. It serves a good purpose for this feature. However, the notation regarding the calories and calorieslist does not seem to be a notation used in CS2113T. Would it be better to use the association lines instead?


The Class diagram for Calories delete feature is shown below:

![CaloriesDeleteClassDiagram](https://github.com/a-wild-chocolate/tp/blob/master/docs/caloriesDeleteUML.jpg)

Choose a reason for hiding this comment

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

The diagram does not seem to appear correctly in the Developer Guide. Would it be better to update the link to a relative path in the code, e.g. "caloriesDeleteUML.jpg" instead of the github link?


The Class diagram for Hydration list feature is shown below. Unrelated attributes and Classes were excluded.

![HydrationListClassDiagram.png](HydrationListClassDiagram.png)

Choose a reason for hiding this comment

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

The association between hydration and hydrationlist does not seem to be a notation used in cs2113. Would it be better to update the diagram to use association lines instead?

Choose a reason for hiding this comment

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

Yeah, this makes sense here.

- Step 5: The latest hydration list will be updated to saving file by calling `HydrationList#updateFile()`.

The Sequence diagram for Hydration delete feature is shown below:
![HydrationDeleteDiagram.png](HydrationDeleteDiagram.png)
Copy link

@itsmejr257 itsmejr257 Apr 4, 2024

Choose a reason for hiding this comment

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

The notation for this sequence diagram appears to be different from the rest of the diagrams. The methods called seem to be indicated with the class. E.g. Ui#handleHydrationinput. Would it be better to standardize the notations to match the previous diagrams, in particular, following the notation taught during lectures?


- Step 5: The latest hydration list will be updated to saving file by calling `HydrationList#updateFile()`.

The Sequence diagram for Hydration delete feature is shown below:

Choose a reason for hiding this comment

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

Great Job at keeping the application used to make the sequence diagrams the same.

**Format:**
`calories list`

#### Sample output
Copy link

Choose a reason for hiding this comment

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

It's good to tell the user about what they can potentially get.

@@ -37,6 +235,17 @@ Example of usage:

## Command Summary

{Give a 'cheat sheet' of commands here}

| Action | Format, Examples |
Copy link

Choose a reason for hiding this comment

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

Good, a help function can always help the user when they get lost.

{Give a product intro}
LifeTrack is a desktop app for students to track their health data, optimized for use via a Command Line Interface (CLI). It tracks calories, hydration and sleep data for the user, while also providing daily recommendations for calorie and hydration intake, based on the user's build and gender, as well as their body goals and activity levels.

## Quick links
Copy link

Choose a reason for hiding this comment

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

A good navigation page

- Step 5: The latest hydration list will be updated to saving file by calling `HydrationList#updateFile()`.

The Sequence diagram for Hydration delete feature is shown below:
![HydrationDeleteDiagram.png](HydrationDeleteDiagram.png)

## Product scope
### Target user profile
Copy link

Choose a reason for hiding this comment

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

Users have various reasons to choose your product, maybe have more lines and contents for user stories?

Copy link

Choose a reason for hiding this comment

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

Is it possible to make the bar thicker, it's kinda hard to distinguish the bars when classes are calling multiple functions inside the class.

- `ParserCalories#parseCaloriesInput(String)`
- `FileHandler#updateFile()`

This feature is activated when the user inputs a `calories in` or `calories out` command in the terminal.
Copy link

Choose a reason for hiding this comment

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

Very detailed step descriptions.

Copy link

@ShyamKrishna33 ShyamKrishna33 left a comment

Choose a reason for hiding this comment

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

Overall, the DG looks great with minor bugs in rendering sequence diagrams.

- Step 5: The latest sleep list will be updated to saving file by calling `SleepList#updateFile()`.

The Sequence diagram for Sleep delete feature is shown below:
![SleepDeleteDiagram.jpg](https://github.com/a-wild-chocolate/tp/blob/f9a94746e763ddf54a4309583b71e0fdbabdb141/docs/SleepDeleteSeqDiagram.jpg)

Choose a reason for hiding this comment

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

The image of the sequence diagram is not rendered correctly.
Screenshot 2024-04-04 152229


The Class diagram for Hydration list feature is shown below. Unrelated attributes and Classes were excluded.

![HydrationListClassDiagram.png](HydrationListClassDiagram.png)

Choose a reason for hiding this comment

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

Yeah, this makes sense here.


The sequence diagram for this feature is shown below:

![CaloriesAddEntrySeqDiagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/rexyyong/tp/RexDG/docs/CaloriesAddEntrySeqDiagram.puml)

Choose a reason for hiding this comment

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

Remember to add colon ':' in front of all instances of a class.
Screenshot 2024-04-04 152804


#### Implementation

This functionality is facilitated by `UI`, `SleepList`, `FileHandler` and `ParserSleep`. It implements one operation, namely:

Choose a reason for hiding this comment

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

Good job with the implementation!

Copy link

@nichyjt nichyjt left a comment

Choose a reason for hiding this comment

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

Hi team,

Good job at making the DG!

Take note of these general comments:

  1. Please take note of the syntax issues with the diagrams. Standardize your diagram tools.
  2. There are some sections of the DG that are incomplete. Please finish them.
  3. The parts where design considerations were added is good. Don't forget to include design considerations and the design of the feature itself (class diagram) as sections.
  4. Upload your diagrams as files, do not use Github blobs (copy paste directly into the Github code editor).

You may refer to the AB3 DG as reference on how a well structured DG looks like as you fix your DG and add more diagrams/content.

@@ -5,9 +5,238 @@
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}
Copy link

Choose a reason for hiding this comment

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

This should be at the bottom; and it is not complete

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

Copy link

Choose a reason for hiding this comment

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

Consider adding in a Table of Contents


{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
#### Implementation
Copy link

Choose a reason for hiding this comment

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

Good that you zoomed into implementation via the header. What happened to the Design part?


The sequence diagram for this feature is shown below:

![CaloriesAddEntrySeqDiagram](http://www.plantuml.com/plantuml/proxy?cache=no&src=https://raw.githubusercontent.com/rexyyong/tp/RexDG/docs/CaloriesAddEntrySeqDiagram.puml)
Copy link

Choose a reason for hiding this comment

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

As a peer mentioned, you are missing the colon.
The actor Bob is a good adition, but is Bob really a good way of describing a User?

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

Copy link

Choose a reason for hiding this comment

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

For all your images, save it as a proper file inside your docs folder. Avoid copy pasting and letting Github render from blobs. This will help in version control as well.

- Step 5: The latest hydration list will be updated to saving file by calling `HydrationList#updateFile()`.

The Sequence diagram for Hydration delete feature is shown below:
![HydrationDeleteDiagram.png](HydrationDeleteDiagram.png)
Copy link

Choose a reason for hiding this comment

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

Same as itsmejr257 pointed out

- Step 5: `FileHandler#updateFile()` is then called to update the data file with the new entry in the `SleepList`.

The sequence diagram for this feature is shown below:
![SleepAddSeqDiagram](https://github.com/a-wild-chocolate/tp/blob/f9a94746e763ddf54a4309583b71e0fdbabdb141/docs/SleepAddSeqDiagram.jpg)
Copy link

Choose a reason for hiding this comment

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

Standardize the tools used for the diagrams.
A few issues:

  1. No return from activation bar

image

  1. Return line not starting at end of activation bar.
    image

  2. User does not need an activation bar

The `printSleepList()` function iterates through the `sleepList` and each entry will call `SleepEntry#toString()` to return its information string to be printed.

The Sequence diagram for Sleep list feature is shown below. Unrelated attributes and Classes were excluded.
![SleepListSeqDiagram](https://github.com/a-wild-chocolate/tp/blob/f9a94746e763ddf54a4309583b71e0fdbabdb141/docs/SleepListSeqDiagram.jpg)
Copy link

Choose a reason for hiding this comment

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

Same comments as per the previous seq diagram critique

The Sequence diagram for Sleep delete feature is shown below:
![SleepDeleteDiagram.jpg](https://github.com/a-wild-chocolate/tp/blob/f9a94746e763ddf54a4309583b71e0fdbabdb141/docs/SleepDeleteSeqDiagram.jpg)

### Calculating sleep requirements for each user (Planning)
Copy link

Choose a reason for hiding this comment

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

May want to rephrase as proposed future feature

Comment on lines 241 to 242
## Product scope
### Target user profile
Copy link

Choose a reason for hiding this comment

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

Missing content

paturikarthik and others added 30 commits April 15, 2024 23:12
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.