[CS2113T-W09-2] iGraduate #4
Conversation
docs/DeveloperGuide.md
Outdated
| - [iGraduate Developer Guide](#igraduate-developer-guide) | ||
| * [1. Introduction](#1-introduction) | ||
| * [2. Setting up, getting started](#2-setting-up-getting-started) | ||
| + [2.1 Terminal](#21-terminal) | ||
| + [2.2 Intellij IDEA (Recommended)](#22-intellij-idea-recommended) | ||
| * [3. Design](#3-design) | ||
| + [3.1 Architecture](#31-architecture) | ||
| + [3.2 UI Component](#32-ui-component) | ||
| + [3.3 Logic Component](#33-logic-component) | ||
| + [3.4 Model Component](#34-model-component) | ||
| + [3.4.1 `module` Package](#341-module-package) | ||
| + [3.4.1.1 `Module` Class](#3411-module-class) | ||
| + [3.4.1.2 `CoreModule` Class](#3412-coremodule-class) | ||
| + [3.4.1.3 `GeModule` Class](#3413-gemodule-class) | ||
| + [3.4.1.4 `ElectiveModule` Class](#3414-electivemodule-class) | ||
| + [3.4.1.5 `MathModule` Class](#3415-mathmodule-class) | ||
| + [3.4.2 `list` Package](#342-list-package) | ||
| + [3.4.2.1 `ModuleList` Class](#3421-modulelist-class) | ||
| + [3.5 Storage Component](#35-storage-component) | ||
| + [3.6 Common Classes](#36-common-classes) | ||
| * [4. Implementation](#4-implementation) | ||
| + [4.1 UI](#41-ui) | ||
| + [4.2 Parser](#42-parser) | ||
| + [4.3 Command](#43-command) | ||
| + [4.4 Module](#44-module) | ||
| + [4.5 ModuleList](#45-modulelist) | ||
| + [4.6 Storage](#46-storage) | ||
| + [4.7 Exception](#47-exception) | ||
| * [Appendix: Requirements](#appendix-requirements) | ||
| + [Product Scope](#product-scope) | ||
| + [Target User Profile](#target-user-profile) | ||
| + [Value Proposition](#value-proposition) | ||
| + [User Stories](#user-stories) |
There was a problem hiding this comment.
Would it be better if use * for unordered list as per SE-EDU's markdown convention?
| and methods applicable to all class objects. It is then inherited by all other child module classes. A class diagram illustrating | ||
| the relationship between the interaction of classes under the module package is shown below. | ||
|
|
||
|  |
There was a problem hiding this comment.
Would this diagram be a bit redundant especially on the list of methods presented given it is exactly the same for all classes 🤔 ? It also looks like being out of date as some methods were not included.
|
|
||
| The sequence diagram below shows the execution of add command in action: | ||
|
|
||
|  |
There was a problem hiding this comment.
For the "cross" you put for AddCommand, would you like to put it lower? It looks like the instance is out of reference even before it returns values for the current diagram.
docs/DeveloperGuide.md
Outdated
|
|
||
| The following is the UML diagrams for update command consisting of updating each flag. | ||
|
|
||
|  |
There was a problem hiding this comment.
Shouldn't these roles and navigability numbers be at the right side (marked by arrow) instead?
docs/DeveloperGuide.md
Outdated
|
|
||
|  | ||
|
|
||
| <sup>***Figure 4.2.1** Sequence diagram of `Parser` class with user input *"done CS1010 -g A"**</sup> |
There was a problem hiding this comment.
Is there an issue with the markdown formatting here? Seems like there are a few extra asterisks around.
docs/DeveloperGuide.md
Outdated
| 1. List incomplete modules available to be marked as done based on prerequisites completed: | ||
| - `available` | ||
|
|
||
| [DIAGRAM] |
There was a problem hiding this comment.
Perhaps it would be better to leave placeholder markings such as these as a comment so it wouldn't be rendered out. This placeholder appears in multiple parts and can be a little jarring for readers.
docs/DeveloperGuide.md
Outdated
| occurrence would be a module named `Software Engineering and Object-Oriented Programming`, which contains | ||
| dashes when the delimiters are used for separating various module information is also a dash. | ||
|
|
||
| Considerations were also given to use more unique delimiters (such as \, `|`, etc.) to avoid accidental parsing |
There was a problem hiding this comment.
Should the \ also be enclosed in backticks here since the | is?
docs/DeveloperGuide.md
Outdated
| - `Model` -> The user data held by the program | ||
| - `Module` -> Holds the information of individual modules. | ||
| - `ModuleList` -> Holds data of all modules of the app in memory. | ||
| - `Storage` -> Reads data from, and writes data to, the hard disk. |
There was a problem hiding this comment.
Not a major issue, but the markdown coding standard seems to favour the use of * over - for unordered lists.
docs/DeveloperGuide.md
Outdated
|
|
||
| Below is a Command class diagram. | ||
|
|
||
|  |
There was a problem hiding this comment.
Due to the width of this diagram, it can be quite hard to read as the words are very small.
There was a problem hiding this comment.
Agreed, perhaps you can try to arrange the subclasses around the abstract class
docs/DeveloperGuide.md
Outdated
| The module list is stored in a storage file named `modules.json` in the `data` folder | ||
| (`<program location>/data/modules.json`). | ||
|
|
||
|  |
There was a problem hiding this comment.
Some of the diagrams (this included) appear different in format/style from the others. Perhaps it would be better to use a consistent format/style across the whole document?
| 2. Then, `removeFromModuleUntakenPrerequisites` is called to remove `existingModule` from the prerequisitesUntaken table | ||
| from the list of modules that require `existingModule` as a prerequisite. | ||
|
|
||
| Given below is the class diagram of `ModuleList` showing its 3 main functions. |
There was a problem hiding this comment.
Are 3 functions sufficient for explanation?
docs/DeveloperGuide.md
Outdated
|
|
||
| Below is a Command class diagram. | ||
|
|
||
|  |
There was a problem hiding this comment.
Agreed, perhaps you can try to arrange the subclasses around the abstract class
| The CAP command calculates the current CAP of the user based on the grades of modules that are marked as done. The | ||
| command also displays the degree classification of the user. There are no flags or additional parameters required. | ||
|
|
||
|  |
There was a problem hiding this comment.
I the diagram slightly overdetalied with the cap command calling itself multiple times?
| 2. Extract the relevant parameters and flags required to run the command | ||
| 3. Create a new `Command` object and hand it over to `iGraduate` to execute | ||
|
|
||
|  |
There was a problem hiding this comment.
Are all the self call methods necessary to be included?
There was a problem hiding this comment.
Agreed, seems like it could be excluded for more readability.
docs/DeveloperGuide.md
Outdated
| + [Progress](#progress) | ||
| + [List Modules](#list-modules) | ||
| + [Saving data](#saving-data) | ||
| ## 1. Introduction |
There was a problem hiding this comment.
Did you intent for introduction to be a main header instead?
|
Should "Introduction" be indented at the same level as the other headers like "Setting up, getting started"? |
|
|
|
|
|
|
|
|
||
| ### 3.1 Architecture | ||
|
|
||
|  |
There was a problem hiding this comment.
Perhaps resize the diagram to match the text size so that it looks more polished?
docs/DeveloperGuide.md
Outdated
|
|
||
|  | ||
|
|
||
| <sup>***Figure 3.3.2.1** UML class diagram for Command class*</sup> |
There was a problem hiding this comment.
Is this the intended size for the diagram? Do you think it's better to split up the diagram into several reference diagram so that developers will have an easier time reading them?
| and methods applicable to all class objects. It is then inherited by all other child module classes. A class diagram illustrating | ||
| the relationship between the interaction of classes under the module package is shown below. | ||
|
|
||
|  |
There was a problem hiding this comment.
Is it more readable if you separate the repeated content into a new reference?
docs/DeveloperGuide.md
Outdated
| and `requiredByModules` module respectively, whereas `getStatusIcon` returns the status icon based on the module | ||
| status. For customized formatting of module printing messages, `toString` method is overridden. | ||
|
|
||
| #### 3.4.1.2 `CoreModule` Class |
There was a problem hiding this comment.
Should you avoid using 'toString' in all of the subclass explanation? Since you already mentioned it in the parent class explanation, is it enough to just stated that all the subclass contains the 'toString' method?
docs/DeveloperGuide.md
Outdated
|
|
||
| Class Diagram: | ||
|
|
||
|  |
There was a problem hiding this comment.
Small issue but do you think it's better to use the same uml diagram style throughout the document?
| 2. Extract the relevant parameters and flags required to run the command | ||
| 3. Create a new `Command` object and hand it over to `iGraduate` to execute | ||
|
|
||
|  |
There was a problem hiding this comment.
Agreed, seems like it could be excluded for more readability.
…into xinRu-Refractor
Update team documentation
Add Project Portfolio Page and Update Documentation
Update PPP and command class diagram in DG
Remove whitespace from DG
…into xinRu-Documentation
Fix small details in Sequence Diagrams
…into xinRu-Documentation
Update author tag and documentation
Final changes
…into xinRu-Documentation
Edit Fuxi PPP
Updated UG Errors
iGraduate is a command-line application that will help NUS students majoring in information security check his/her graduation progress and modules taken in a coherent manner based on the programme requirements.