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

[CS2113T-F11-1] nocap #44

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

Conversation

jiexiong-zeng
Copy link

NoCap (NC) is a desktop app for managing modules taken in NUS, optimized for use via a Command Line Interface (CLI). If you can type fast, NC can get your module management tasks done faster than traditional GUI apps. It is the perfect app for NUS students!


**API** : module

Data from all the modules are stored in the ModuleList class. This includes:
Copy link

Choose a reason for hiding this comment

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

Maybe could add markups for ModuleList and ScheduleList as well to standardize with the rest of the components


**API** : `semester`

![alt_text](media/SemesterListDiagram.jpg)
Copy link

Choose a reason for hiding this comment

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

Should the arrow from module list to module have a diamond at module cause it seems like an aggregation or composition of module objects
Screenshot 2021-10-27 at 11 36 31 AM


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

Choose a reason for hiding this comment

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

Maybe add a subheading on how NoCap as a whole works like how you've done for parser. As in a program flow from user input till the output returned to user, showing how all the components work together.




1. `TaskList` stores all tasks in an `ArrayList<Task>`.
Copy link

Choose a reason for hiding this comment

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

Screenshot 2021-10-27 at 11 54 00 AM

The formatting has broken slightly over here. Also is removeDate() responsible for getting description, or is it a typo?

Comment on lines 72 to 73
Step 5: `overallListParser` creates an `OverallTaskList`. Through nested switch cases, **TaskType** and **
TaskDescription** are matched, and the corresponding method `OverallTaskList#sortByDateAndPrint()` is called. As the

Choose a reason for hiding this comment

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

image
Appears incorrectly as TaskType and ** TaskDescription**.

Comment on lines 179 to 180
- If day of week and timeslot corresponds, venue and comments information is printed out
- If day of week and timeslot does not correspond, and blank character " " is printed instead.

Choose a reason for hiding this comment

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

Formatting here seems off. Appears like this:
image

Comment on lines 201 to 207
day of week

timeslot

location

comments

Choose a reason for hiding this comment

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

Could format this better like this section for consistency:
image
Currently appears like this:
image

Comment on lines 166 to 167
<<<<<<< HEAD
The modules are stored in an ArrayList and ModuleList uses the Module.get(int index) method to access the target Module.

Choose a reason for hiding this comment

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

Formatting is incorrect here. Appears like this:
image

`list task sortbydate`.

The Sequence Diagram below illustrates the process
![alt_text](media/ParserSequenceDiagram.png)

Choose a reason for hiding this comment

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

I think the group missed out on the colon : on the entity boxes for this sequence diagram

image


The diagram below illustrates the `splitString` process:

![alt_text](media/splitStringDiagram.JPG)

Choose a reason for hiding this comment

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

Perhaps using a sequence diagram here is better? To make it more consistent with the diagram types you have been using throughout the DG.
Other developers may have different interpretations for non standard diagrams

image


![alt_text](media/capComputationSequenceDiagram.png)

* When `commandAddGrade()` or `commandAddCredit()` is called in Parser, `addgrade(description)`

Choose a reason for hiding this comment

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

perhaps the group meant addGrade(description) here instead of addgrade(description)


All data related to module is stored in the module class. An Arraylist of Module is used to store and manage the modules. ModuleList is also responsible for constructing and printing out the Timetable.

![moduleListClassDiagram](media/moduleListClassDiagram.png)

Choose a reason for hiding this comment

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

There is a minor formatting error in this diagram for #int: credits

image

- ModuleList contains the getter method find(String input) which returns a module by the same name as the input.
- Module contains getter and setter methods to change or access its contents.
- When Module is constructed, an empty gradableTaskList, taskList and scheduleList wll be instantiated and stored in Module.

Choose a reason for hiding this comment

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

Perhaps the use of markdown here will make it clearer here.

image

It appears that the use of markdown is minimal in this section here. The group may want to consider using more markdowns for this section

- If day of week and timeslot corresponds, venue and comments information is printed out
- If day of week and timeslot does not correspond, and blank character &quot; &quot; is printed instead.
=======
# ![modulePrintTimetableSeq](media/modulePrintTimetableSeq.png)

Choose a reason for hiding this comment

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

The group perhaps forget to include colon : for the entity boxes in this sequence diagram.

image

list.
3. toString() prints out all relevant schedule information in a list format. This is done by going through the list and
printing Schedule one after another.

Choose a reason for hiding this comment

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

Perhaps more markdowns can be used here to differentiate a code from a normal text

image

characters in length. This is to ensure that it fits within its time slot within the Timetable when printed.

![scheduleseq](media/scheduleseq.png)

Choose a reason for hiding this comment

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

Colon : is missing here for the entity boxes

image

Copy link

@mohamedirfansh mohamedirfansh left a comment

Choose a reason for hiding this comment

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

The overall DG looks good!


{Describe the value proposition: what problem does it solve?}
![alt_text](media/ParserClassDiagram.JPG)

Choose a reason for hiding this comment

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

The high level class diagram is a good addition for the reader to understand the overall structure of the parser. But perhaps you could try to follow the same style of class diagram that you have used for Semester.


**API** : `semester`

![alt_text](media/SemesterListDiagram.jpg)

Choose a reason for hiding this comment

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

image
This might perhaps be a aggregation relation instead as there seems to be a list of semester objects inside the semesterList class.


**API** : `task.tasklist`

![alt_text](media/TaskClassDiagram.png)

Choose a reason for hiding this comment

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

image
The self calling arrows here on TaskList are a little confusing to understand at first glance. You could perhaps extend the length of the activation block so that the arrows could be more spaced out.

Comment on lines 269 to 276
1. Whenever the `Task` object is instantiated, the `attributes` listed above will be initialized by the `setter`
methods: `setDescription()`, `setDate()`, `setDone()`, `setLate()` and `setDeadline()`.
2. When calling `printAllTask()`, `printWeeklyTask()`, `printMonthlyTask()` in `OverallTaskList` the
method `updateOverdue()`will be called which checks for the truth value of the `boolean` attribute `isDone` and also
whether the current date and time of the system clock is after the `deadline` of the `Task` object.
3. If `isDone` is `FALSE` and the `deadline` is later than the current date and time, `updateOverdue()` will set the
attribute `isLate` of the current `Task` object to `TRUE`.
4. Calling the toString prints out the task information in the Task object.

Choose a reason for hiding this comment

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

You could perhaps draw a sequence diagram for this set of steps just like how you have done for the others to help with clarity.

DmitriYam and others added 26 commits November 1, 2021 18:27
* 'master' of https://github.com/AY2122S1-CS2113T-F11-1/tp:
  add /m module list gradable
  add /m module list gradable
  Remove magic number
  fix checkstyle issues
  Update UG to give clearer explanation on list task functions
  Updated UG
  Edit UG
  Updated UG
  Bug fixes
  checkstyle
  fix bugs mainly in UG
  UG updates
Add grade bug and UG updates
- Added list task normal
* 'master' of https://github.com/AY2122S1-CS2113T-F11-1/tp:
  Add ug list task section
  fix bug
  fix checkstyle
  fix checkstyle
  fix functional bugs
  addgrade bug fix
Add documentation for parser methods
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.

9 participants