Skip to content

[CS2113-T12-4] WellNUS++#25

Open
wenxin-c wants to merge 1256 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T12-4:master
Open

[CS2113-T12-4] WellNUS++#25
wenxin-c wants to merge 1256 commits into
nus-cs2113-AY2223S2:masterfrom
AY2223S2-CS2113-T12-4:master

Conversation

@wenxin-c

@wenxin-c wenxin-c commented Mar 2, 2023

Copy link
Copy Markdown

WellNUS++ is a Command Line Interface(CLI) app for NUS Computing students to keep track and improve their physical and mental wellness in various aspects. If you can type fast, WellNUS++ can update their wellness progress faster than traditional Graphical User Interface(GUI) apps.

@ngkaiwen123 ngkaiwen123 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Description of design and implementation is generally well done! Good work! Some diagrams may need to be improve for better readability.

Comment thread docs/DeveloperGuide.md Outdated
### Reflection Component

![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png)
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.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
The dotted arrows appear to be a little confusing. Perhaps it would be possible to redraw them?

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +218 to +243
`LikeCommand` class: <br>

- Command format: `like <index of question(1~5)>`
- Users can add an introspective question that is generated in the previous set into their favorite list. Since there
will only
be 5 questions per set, the indexes are restricted to integer 1~5.
- `addFavQuestion()` method in `QuestionList` class is used to add and store the data.
- Users can only successfully add a question to favorite list if they have gotten a set of questions previously.
- Every time a question is added into the favorite list, the indexes of this particular question will be stored in data
file straightaway. It prevents data loss due to unforeseen computer shutdown.

`FavoriteCommand` class: <br>

- Command format: `fav`
- Users can get the questions in their favorite list.
- `getFavQuestions()` method in `QuestionList` class matches the indexes to corresponding questions and return this set
of questions
back to `FavCommand` for output. As such, `QuestionList` is a dependency for `FavoriteCommand` as well.

`HomeCommand` class: <br>

- Command format: `home`
- This command allows users to return back to the main WellNUS++ interface.
- Similar to `GetCommand`, `validateCommand()` method will also be called to validate the command.
- It will then call the class-level method `ReflectionManager.setIsExit()` to terminate the while loop
in `Reflectionmanager`.

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 it will also be great to include how these classes handle errors? Or if there are any cases (or zero cases) of errors they may need to handle?

Comment thread docs/DeveloperGuide.md
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
Implementation of `parseUserInput`:

![CommandParser implementation](diagrams/CommandParserSequence.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
A reference frame for the CommandParser accessing its own methods inside the loop may help to better describe what is happening in the sequence diagram?

Comment thread docs/DeveloperGuide.md Outdated

### AtomicHabit Component

![AtomicHabit Component](diagrams/AtomicHabit.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
A redraw of the arrows to make them seem neater may be better for the reader?

Comment thread docs/DeveloperGuide.md
#### Overview
The overall execution lifecycle of the WellNus application involves 4 main components, as shown in the diagram below.

![Application Lifecycle](diagrams/WellnusSequence.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
Text on dotted lines should be the variable returned rather than a description?

@ngyida ngyida left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice DG overall! The link to github is a nice touch :)

Comment thread docs/DeveloperGuide.md Outdated

![Application Lifecycle](diagrams/WellnusSequence.png)

The application begins with a call to `WellNus.start()`, which initialises an instance of `MainManager` and calls the

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could this be done without since it is a description of the sequence diagram?

Comment thread docs/DeveloperGuide.md Outdated
Comment on lines +132 to +133
![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png)
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.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.

This diagram is too crowded which makes it hard to read. Could you perhaps abstract some of the details out?

Comment thread docs/DeveloperGuide.md Outdated
are not the focus of this section since they are outside of `reflection` package.<br>
<br>

#### Feature Package (`ReflectionManager`, `ReflectionQuestion`, `QuestionList`, `TextUi`, `RandomNumberGenerator` classes)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could consider the use of UML diagrams to represent class to improve clarity

Comment thread docs/DeveloperGuide.md

#### Integration with WellNUS++

![Integration](diagrams/CommandParserClass.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.

This class diagram may not follow the standard UML notation

Comment thread docs/DeveloperGuide.md Outdated
### Reflection Component

![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png)
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.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.

The class diagram here could possibly be abstracted out and separated out into different features since it is not mandatory to include everything. Users may find it hard to read as the text size is a little small

Comment thread docs/DeveloperGuide.md Outdated

### AtomicHabit Component

![AtomicHabit Component](diagrams/AtomicHabit.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.

To simplify the diagram, maybe not every command has to be shown, which would definitely reduce the amount of arrows

Comment thread docs/DeveloperGuide.md Outdated
The payload will terminate when the user clicks `enter` or separates the payload with another argument
with the `--` delimiter.

## Instructions for manual testing

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this section could be removed from the developer guide? This is because your user guide already covers the commands, creating duplicate content.

Comment thread docs/DeveloperGuide.md Outdated

For example,
`
$ foo bar --arg1 payload1 payload1--1 --arg2 payload2 --arg3

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this could be included in a code block for easier visualization

Comment thread docs/DeveloperGuide.md
* The app is gamified to make it more attractive for students to use. Levels and micro-goals incentivise our
users to keep using the app’s features, allowing them to focus on their work and achieve wellness.

## User Stories

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

User stories language format could be changed to fit the "I want to ..." sentence. For e.g. I want to use keyboard instead of mouse vs I want to I can use keyboard instead of mouse

Comment thread docs/DeveloperGuide.md Outdated
### Reflection Component

![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png)
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.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.

diagram is messy and hard to visualise. might want to consider removing some of the less important parts and only showing the important features of the diagram

Comment thread docs/DeveloperGuide.md Outdated
| v2.0 | Reflective student | I can get the previous questions I viewed | I can re-view these questions |
| v2.0 | Easily distracted computing student | I want to start a timer to keep track of time spent on work | I can do timed-practice |
| v2.0 | Easily distracted computing student | I want to check the time | I can keep track of my pace |
| v2.0 | A regular WellNUS++ user | I wish to have my information stored in the app | I can re-view my past data |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can add priority levels / level of importance to this

@Stella1585 Stella1585 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(sorry i forgot to submit my review)
developer guide is very informative!

Comment thread docs/DeveloperGuide.md Outdated

### Reflection Component

![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.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 feel that this might be a little repetitive as each command is executed in a similar way. It also ends up being very long and looks complicated. Maybe you can separate the commands.

Comment thread docs/DeveloperGuide.md Outdated
### Reflection Component

![Reflection Component Class Diagram](diagrams/ReflectionSequenceDiagram.png)
![Reflection Component Class Diagram](diagrams/ReflectionClassDiagram.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.

The association lines are all overlapping, and it is difficult to tell them apart, especially at the top where the label and lines overlap. once again i suggest you can separate into perhaps feature and command diagrams.

Comment thread docs/DeveloperGuide.md Outdated
The application begins with a call to `WellNus.start()`, which initialises an instance of `MainManager` and calls the
`MainManager.runEventDriver()` method.

`MainManager.runEventDriver()` will then take control of user input and provide a basic interface that parses commands

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are mentions of keywords and user inputs that are being tackled but i do not see a string being passed through or a text ui class, maybe u can show this in your digram too?

Comment thread docs/DeveloperGuide.md
{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
Implementation of `parseUserInput`:

![CommandParser implementation](diagrams/CommandParserSequence.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.

Well made sequence diagram, helps me to understand how a command is parsed.

YongbinWang and others added 25 commits April 8, 2023 19:38
Replace 'Implementation Rationale' with
  'Design Considerations' to standardise
  these section headers.
wenxin-c and others added 30 commits April 10, 2023 20:25
…rror

Remove extra space printed for MainManger when invalid command is given
…Parser

Fix developer guide typos command parser
Fix single-letter variable name
Update DeveloperGuide Table of Contents
Fix list indices in DeveloperGuide
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