-
Notifications
You must be signed in to change notification settings - Fork 248
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-T15-1] CLIAlgo #33
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DG is pretty nice and clear overall!
docs/DeveloperGuide.md
Outdated
### Initializing previous saved data feature | ||
#### Current implementation | ||
|
||
![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the activation bar for the FileDecoder
a bit too long?
docs/DeveloperGuide.md
Outdated
### Export feature | ||
#### Current implementation | ||
|
||
![](.\\sequence\\diagrams\\Export.png "Export Sequence Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can explain more about the part of Parser
to FilterByTopicCommand
to TopicManager
, since they are shown on the diagram. Or perhaps it is not that important and can be ignored in the sequence diagram.
docs/DeveloperGuide.md
Outdated
Here is a class diagram of the `TopoCommand` which facilitates the storage | ||
function of the application. | ||
|
||
![](.\\uml\\diagrams\\TopoCommandClass.png "TopoCommand Class Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FileManager
doesn't seem to be very relevant to the class diagram, and it makes the lines crossing each other. Perhaps it doesn't have to be in this diagram.
#### Current implementation | ||
|
||
![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you might need an arrow back from SingleFile
back to FileManager
after the readFile()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DG is detailed and easy to read! Great job 👍
docs/DeveloperGuide.md
Outdated
|
||
### Ui | ||
**API** : Ui.java | ||
Here is a class diagram of the `Ui` component which is responsible for handling all interactin with the User. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a small typo here ("interactin")
docs/DeveloperGuide.md
Outdated
|
||
The following sequence diagram shows how the Parser work. | ||
|
||
![](.\\sequence\\diagrams\\Parser.png "Parser Sequence Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The step-by-step description of the Parser is easy to understand! Perhaps the sequence diagram can also be simplified such that it is easier to comprehend together with the description (e.g. by removing return arrows, which is easy to infer)? This could also be applied to the sequence diagrams for filtering by topic operation.
docs/DeveloperGuide.md
Outdated
> **Step 2**: The `parse()` method extracts out the command keyword provided by the user. It then calls the | ||
> `prepareCommand()` method. | ||
|
||
> **Step 3**: The `parepareCommand()` identifies the correct `Command` object to prepare based on the command keyword. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a small typo here ("parepareCommand()")
@@ -2,37 +2,615 @@ | |||
|
|||
## Acknowledgements | |||
|
|||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} | |||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the | |||
original source as well} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great if there was a link to your UG/a brief overview of what this app is programmed to do! It would make it easier see all the following technical details in the bigger picture.
docs/DeveloperGuide.md
Outdated
Here is a class diagram of the `FileManager` which facilitates the storage | ||
function of the application. | ||
|
||
![](.\\uml\\diagrams\\FileManagerClass.png "FileManager Class Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the diagram's focus would be clearer without the Scanner
and FileWriter
classes, as their functions can be described using labels.
@@ -2,37 +2,615 @@ | |||
|
|||
## Acknowledgements | |||
|
|||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} | |||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the | |||
original source as well} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to include a table of contents and links to the headers for easier navigation
docs/DeveloperGuide.md
Outdated
|
||
The following sequence diagram shows how the Parser work. | ||
|
||
![](.\\sequence\\diagrams\\Parser.png "Parser Sequence Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/DeveloperGuide.md
Outdated
|
||
The following sequence diagram shows how previously saved files are loaded into `CLIAlgo`. | ||
|
||
![](.\\sequence\\diagrams\\InitializationFileManager.png "FileManager Initialization Sequence Diagram") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be try to modify the diagram to ensure the lines are not crossing each other
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The activation bar of [isCorrupted] should not be disconnected
docs/DeveloperGuide.md
Outdated
@@ -2,37 +2,614 @@ | |||
|
|||
## Acknowledgements | |||
|
|||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well} | |||
{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this section have links to your references?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the AddCommand and InvalidComment be shown at the end of the diagram when a red cross deletes the instance? This applied similarly to your other sequence diagrams.
docs/UML/diagrams/ParserClass.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the command class also have two blank sections for attribute and method like other classes?
|
||
1. Ensure that you have Java 11 or above installed. | ||
1. Down the latest version of `Duke` from [here](http://link.to/duke). | ||
## Features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the features section be placed in the user guide instead?
Refactor code v2.1
…into branch-Code-Review-Fix-v2.1
…into branch-v2.0b-DG # Conflicts: # docs/sequence-diagrams/diagrams/AddFeature.png # docs/sequence-diagrams/source-files/AddFeature.puml
Add design and implementation for remove and update add sequence diagram
…into branch-PPP-v2.1
Code refactoring v2.1
Refactor DG and UG
…into branch-Diagrams-v2.1
Branch final edits v2.1
…into branch-Final-Refactor-v2.1
Final Refactor of UG, DG and PPP v2.1
Branch final changes v2.1
Fix Portfolio Link in AboutUs
Reformat table in AboutUs.md
Fix bug in TopoCommand
CLIAlgo is a desktop application for managing your CS2040C notes. It is optimized to be used via a Command Line Interface (CLI). If you can type fast, you can access and sort your notes faster than ever before.