[CS2113-T14-3] termiNus by GuoAi · Pull Request #43 · nus-cs2113-AY2021S1/tp

GuoAi · 2020-10-01T06:35:08Z

A software for NUS University undergraduate students.

…l-Feature1 Manuel feature1

LIU-YiFeng-1

Good job! easy to read and understand!

LIU-YiFeng-1 · 2020-10-29T04:34:29Z

+Below is an architecture diagram of termiNus.
+
+![ArchitectureDiagram](./images/ArchitectureDiagram.png)
+
+- `Duke` is the main object of the program and handles all the logic and models related to the program.
+- `Ui` is the main object that provides an interface between `Duke` and the user.
+- `Command` represents a command that is provided by the user. `Ui` reads the command and it is sent to `Parser` to create a new `Command` object.
+- `Duke` executes the command and shows the output to the user using `Ui`.
+- `Command` object modifies the state of `ItemList`, which consists of multiple lists for different types of `Items`.
+- `Storage` takes the state of `ItemList` and stores it to file.


Good job, I like how the architecture diagram is used here. Easy to understand

I think its a good idea to have this diagram first but it may be a bit confusing to those looking at it for the first time. Maybe adding some colour to help differentiate the different parts would be good? I also noticed that you did not describe all the items in the diagram in the explanation, if these are the main components of your architecture then maybe you can narrow it down to this as well?

LIU-YiFeng-1 · 2020-10-29T04:37:12Z

+The following sequence diagram shows how the `Parser` works.
+
+![DukeSequenceDiagram](./images/ParserSequenceDiagram.png)
+


It seems that the sequence diagram is not being terminated, is it how it supposed to be? Correct me if I am wrong

LIU-YiFeng-1 · 2020-10-29T04:40:23Z

+Two of the previously mentioned methods, `removeArgumentsFromCommand` and `getArgumentsFromRegex` make use of regular
+expressions to parse the optional arguments.
+
+- The regular expression that parses these optional arguments is `([\w]+/[^\s]+)`. This regular expression matches 1 or more
+alphanumeric characters (denoted by `[\w]+`), followed by a forward slash, then 1 or more of any character except whitespace (denoted by `[^\s]+`).
+- The expression also uses capturing parenthesis to ensure that the parser does not parse the same argument twice.


Is it possible to provide an example in the form of a picture? It is quite confusing to a first-time reader, given that there are so many different symbols.

LIU-YiFeng-1 · 2020-10-29T04:41:57Z

+Methods handling data loading (i.e. `loadTask()`, `loadBook()`, `loadLinks()`, `loadCredit()`, `loadModule()` methods) 
+return an `ArrayList` of `Item`s (i.e. `Task`, `Book`, `Link`, `Credit`, `Module`). These will be the initial values of 
+the `ItemList`. The `save()` method takes an `ItemList` and a `String` specifying the path to which the file will be 
+saved. The `ItemList` will be parsed and saved into files (each `ItemList` will be saved to a separate file) at the 
+specified path.  


perhaps a class diagram to aid the explanation?

LIU-YiFeng-1 · 2020-10-29T04:48:15Z

+There are 6 fields stored for each `Task`:  
+1. String `T` for "Task"  
+2. Whether the `Task` has been done or not (1 for done, 0 for not done)  
+3. Description of the `Task`  
+4. Priority of the `Task` (an Integer)
+5. Category of the `Task`
+6. Date of the `Task`  
+
+All the fields are separated by ` | ` with a leading and a trailing space. Each `Task` is stored as one line.  
+
+Example: `T | 0 | borrow book | 1 | book | 28-10-2020`  


Good job! I like how this format is used consistently to explain the store files.

LIU-YiFeng-1 · 2020-10-29T04:52:23Z

+|Version|Priority| As a ... | I want to ... | So that I can ...|
+|--------|----------|----------|---------------|------------------|
+|v1.0|***|student|add tasks into a list|keep track of the things I need to do|
+|v1.0|***|student|assign priorities to tasks|focus on the more important things first|
+|v1.0|**|student|assign categories to tasks|have a more organised task list|
+|v1.0|***|student|mark tasks as done|keep track of the remaining tasks to do|
+|v1.0|**|student|list all tasks in my list|have a better overview|
+|v1.0|***|student|be able to delete unwanted tasks|focus on the tasks which I need|
+|v1.0|***|student|save all data after using the application|retrieve the data upon running the application
+|v2.0|**|student|automatically create folders for my modules|I do not have to manually create them|
+|v2.0|***|student|add recurring tasks|avoid adding the same tasks every week
+|v2.0|***|student|have a calendar|I can view my current and upcoming tasks
+|v2.0|***|student|be able to set a tracker my borrowed books|avoid overdue fines
+|v2.0|**|student|sort my tasks based on highest priority|focus on those tasks first
+|v2.0|***|student|save zoom links in a centralized place|easily attend my online classes instead of looking through my email for the link 
+|v2.0|***|student|add modules and calculate my CAP|have a better projection of my grades and efforts
+|v2.0|*|student|login with a password|my system is protected 


Perhaps the Priority could be explained in terms of HIGH MEDIUM or LOW? Without the legend to explain the number of *, could be confusing

TomLBZ

Some comments

TomLBZ · 2020-10-29T07:32:55Z

+
+The following sequence diagram shows how the `Storage` works.
+
+![StorageSequenceDiagram](./images/StorageSequenceDiagram.png)


Does this mean save() can only be executed once throughout the program? Correct me if I'm wrong.

I think when an object is destroyed the dotted line should not continue anymore.

TomLBZ · 2020-10-29T07:43:22Z

+##### Sequence Diagram
+Below is a sequence diagram of how the main program functions.
+
+![DukeSequenceDiagram](./images/DukeSequenceDiagram.png)


This diagram is inconsistent with the next diagram about storage:

This diagram shows that save() is executed repeatedly but Storage is a singleton in this process.

However, this diagram shows Storage object being distroyed after one call to save().
Is one of the diagrams wrong?

TomLBZ · 2020-10-29T07:45:45Z

+##### Sequence Diagram
+Below is a sequence diagram of how the main program functions.
+
+![DukeSequenceDiagram](./images/DukeSequenceDiagram.png)


Is ItemList supposed to keep existing when Duke is destroyed?

TomLBZ · 2020-10-29T07:47:46Z

+
+The following sequence diagram shows how the `Parser` works.
+
+![DukeSequenceDiagram](./images/ParserSequenceDiagram.png)


Should the command be destroyed here?

kstonekuan · 2020-10-30T04:10:06Z

+- First, the `main` function of the `Duke` class creates an instance of `Duke`.
+- During the instantiation of `Duke`, a `Storage` object is created.
+- `Duke` creates and loads the state of `ItemList` from file by calling a function from `Storage`.
+- After `Duke` is initialized, the `Duke` class calls the `run()` method of `Duke`.
+- `Duke` calls methods from `Ui` class and shows messages to the user.
+- `Duke` reads commands from the user using the `Ui` class (which acts as an interface between `Duke` and the user).
+- `Command` object is returned to `Duke` which is executed.
+- `Command` object interacts with `ItemList` and changes its state.
+- `Storage` saves the state of `ItemList` to file.
+- `Duke` continues reading commands until a `ByeCommand` is generated by the user.


Good explanation but if this is a step by step process, maybe you can use a numbered list instead?

kerct · 2020-10-30T04:15:37Z

Should this section be filled up too?

kerct · 2020-10-30T04:18:41Z

Would it be better to explain the optional arguments here too? For example, what does the p in p/0 mean?

kerct · 2020-10-30T04:22:34Z

Should this section be filled up too?

Same for the Command component!

kerct · 2020-10-30T04:24:55Z

Perhaps it would be neater if the examples are formatted in a list rather than listing it out in one line?
For example, the methods handling data loading are:

loadTask()
loadBook()
loadLinks(), etc.

kstonekuan

Heading in the right direction with the sequence diagrams and detailed explanations but the organisation of the sections is a bit off and need more class diagrams help with understanding the classes.

kstonekuan · 2020-10-30T04:16:01Z

+
+#### High level description
+
+The `Parser.parse()` method takes in `fullCommand` as an argument, which is the full command entered by the user.


While this is a good explanation I think it would be good to have a class diagram here as well to show the relationship

kstonekuan · 2020-10-30T04:17:53Z

+An optional argument consists of 2 parts, which is delimited by a forward slash. In the example above, there are 3 optional
+arguments, which are `mc/4`, `ay/2021S1` and `g/A`. Each optional argument can be represented in this form: `<key>/<value>`.


This is a good explanation and makes it clear, especially <key>/<value>. Maybe it would be worth it to explicitly list out the keys as being mc, ay and g and what they each mean even if it can be inferred.

kstonekuan · 2020-10-30T04:22:39Z

+
+The following sequence diagram shows how the `Storage` works.
+
+![StorageSequenceDiagram](./images/StorageSequenceDiagram.png)


I think when an object is destroyed the dotted line should not continue anymore.

kstonekuan · 2020-10-30T04:25:38Z

+|v2.0|***|student|add modules and calculate my CAP|have a better projection of my grades and efforts
+|v2.0|*|student|login with a password|my system is protected 
+
+## Implementation


I feel it is a bit confusing that there is another 'Implementation' section here when there were many implementations described in the section above under 'Design & Implementation', maybe this should be moved to there as well? I believe this would go under the Command component.

kerct · 2020-10-30T04:35:43Z

I feel that putting each file (tasks.txt, books.txt, links.txt and modules.txt) as a subsection rather than a bulleted list would be more appropriate here!

kerct

Overall, the developer guide is very detailed. It might be helpful to include a contents overview at the start with links to the relevant sections too.

kerct · 2020-10-30T04:43:34Z

Just a small grammar issue here - it should be "forget" and "want", not "forgets" and "wants".

max-wunan · 2020-10-30T06:32:45Z

 # Developer Guide

 ## Design & implementation
+<!-- @@author iamchenjiajun -->


You guys may consider add a content table with hyperlink and an introduction for your product and this document (developer guide), so that the readers can have a rough idea of your whole project and can choose to look into detail in specific sessions, instead of being thrown with architecture content/diagram right at the beginning.

Remove unused code and rename variables/methods

…into jiajun-improveDocs

…d spacing)

Update DG

DG user stories

Fix error with category command

Fix errors with Commands

Fix DG bug

minor fixes

Fix error

fix minor issues in DG and UG

Refactor

update

Update PPP to fit page limit

Allupdates

typo in DG

Cut PPP

yeapcl added a commit to yeapcl/tp that referenced this pull request Oct 13, 2020

Merge pull request nus-cs2113-AY2021S1#43 from manuelmanuntag96/manue…

4a94674

…l-Feature1 Manuel feature1

LIU-YiFeng-1 reviewed Oct 29, 2020

View reviewed changes

TomLBZ reviewed Oct 29, 2020

View reviewed changes

kstonekuan reviewed Oct 30, 2020

View reviewed changes

kerct reviewed Oct 30, 2020

View reviewed changes

max-wunan reviewed Oct 30, 2020

View reviewed changes

MuhammadHozefa and others added 17 commits November 8, 2020 14:23

add glossary

8181f92

Add class diagram

f6905fb

Update AboutUs

aba4ddf

updates to DG

3ff3e6b

Merge pull request #180 from iamchenjiajun/jiajun-improveCode

9a61fb7

Remove unused code and rename variables/methods

Merge branch 'master' of https://github.com/AY2021S1-CS2113-T14-3/tp …

caeb961

…into jiajun-improveDocs

Add manual testing.

f6a634e

Add ModelClassDiagram

a72a68e

Add explanation for Model

822a080

Fix DG and update PPP

7ba92d1

Fix output format of list modules and delete tasks with p/ or c/. (ad…

35696e9

…d spacing)

Add explanation for Ui component

d632af6

Add explanation for Command

2b369e1

Merge pull request #185 from iamchenjiajun/jiajun-improveDocs

f712665

Update DG

Merge branch 'master' into DG_user_stories

8af687d

Merge pull request #184 from MuhammadHoze/DG_user_stories

f0c2d4f

DG user stories

Merge branch 'master' into zeyu-updateDG

037e9b4

Cao-Zeyu and others added 30 commits November 9, 2020 22:53

Update notice for find and fix the quick command table for list

4a44da2

Fix CalendarCommand

05c11d3

Fix AddrCommand

42a3ba0

Merge pull request #221 from GuoAi/master

f152c73

Fix error with category command

Merge pull request #222 from iamchenjiajun/jiajun-BugFix

e91618d

Fix errors with Commands

minor errors

ff5002b

checkstyle errors

b8a87d1

Fix DG bug

59a537d

Merge pull request #224 from iamchenjiajun/jiajun-BugFix

070363c

Fix DG bug

io error

c324ef0

Fix error

3953011

fix typos

50f4d86

Merge pull request #223 from MuhammadHoze/Allupdates

590afec

minor fixes

Merge pull request #225 from GuoAi/master

5b1a8ad

Fix error

Merge pull request #226 from Cao-Zeyu/tests

f045351

fix minor issues in DG and UG

Refactor

79a0705

Merge pull request #227 from GuoAi/master

3f197a1

Refactor

Merge pull request #65 from AY2021S1-CS2113-T14-3/master

b415798

update

Update PPP to fit page limit

e96dcfb

fix error msg

bae3a54

typo in DG

8dbac7c

Merge pull request #228 from GuoAi/master

cacbb09

Update PPP to fit page limit

Merge pull request #229 from MuhammadHoze/Allupdates

ec16ff9

Allupdates

Merge pull request #230 from Cao-Zeyu/master

48186b1

typo in DG

Cut PPP

06f345d

Merge pull request #231 from GuoAi/master

dd7753b

Cut PPP

Cut PPP

15d91e0

Merge pull request #232 from GuoAi/master

0f40e6e

Cut PPP

Cut PPP

f80b79d

Merge pull request #233 from GuoAi/master

b5f0a6f

Cut PPP

		The following sequence diagram shows how the `Parser` works.

		![DukeSequenceDiagram](./images/ParserSequenceDiagram.png)


		The following sequence diagram shows how the `Storage` works.

		![StorageSequenceDiagram](./images/StorageSequenceDiagram.png)


		#### High level description

		The `Parser.parse()` method takes in `fullCommand` as an argument, which is the full command entered by the user.

		An optional argument consists of 2 parts, which is delimited by a forward slash. In the example above, there are 3 optional
		arguments, which are `mc/4`, `ay/2021S1` and `g/A`. Each optional argument can be represented in this form: `<key>/<value>`.

Uh oh!

Conversation

GuoAi commented Oct 1, 2020

Uh oh!

LIU-YiFeng-1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomLBZ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kerct commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kerct commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kerct commented Oct 30, 2020

Uh oh!

kerct commented Oct 30, 2020

Uh oh!

kstonekuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kerct commented Oct 30, 2020

Uh oh!

kerct left a comment

Choose a reason for hiding this comment

Uh oh!

kerct commented Oct 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

kerct commented Oct 30, 2020 •

edited

Loading

kerct commented Oct 30, 2020 •

edited

Loading