-
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-F13-3] NUSplanner #52
base: master
Are you sure you want to change the base?
Conversation
Resolve merge conflict in nus-cs2113-AY2223S2#51
docs/DeveloperGuide.md
Outdated
|
||
#### How the feature is implemented: | ||
|
||
![Storage Class Diagram](UML/Images/StorageSequenceDiagram.png) |
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 diagram is good but it looks quite hectic. Consider breaking it up using reference frames with proper annotation for easier comprehension
|
||
The class diagram below illustrates the structure of the storage package | ||
|
||
![Storage Class Diagram](UML/Images/StorageClass.png) |
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.
Simple and easy to read !
|
||
The class diagram below illustrates the structure of the EventList component. | ||
|
||
<img src="UML\Images\EventListUML.png" /> |
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.
Perhaps you should consider noting down the methods within the UI and Parser classes
docs/DeveloperGuide.md
Outdated
|
||
And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted. | ||
|
||
<img src="UML\Images\EventListSD.png" style="zoom:80%;" /> |
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.
Good job nice and easy to read !
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.
Overall the diagrams are very detailed and the explanations were easy to understand.
<img src="UML\Images\EventListUML.png" /> | ||
|
||
And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted. | ||
|
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.
![Storage Class Diagram](UML/Images/StorageClass.png) | ||
|
||
#### How the feature is implemented: | ||
|
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.
![Storage Class Diagram](UML/Images/StorageClass.png) | ||
|
||
#### How the feature is implemented: | ||
|
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.
<img src="UML\Images\EventListUML.png" /> | ||
|
||
And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted. | ||
|
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.
Very concise and clear explanation of your features! Just some minor syntax issues with the UML diagrams. Great work! :)
docs/UML/addEvent.puml
Outdated
@@ -0,0 +1,39 @@ | |||
@startuml | |||
Actor -> Parser : parseAddCommand() |
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 should add a activate Parser
line since the activation bar is missing from your diagram?
-> ":Storage" : loadModules() | ||
activate ":Storage" | ||
Alt modulesLoaded | ||
":Storage" --> ":Storage" : :HashMap<String, NusModule> |
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 this should be a solid arrow since it's a method invocation?
end | ||
":EventListStorage" -> ":FileWriter" : taskWriter.write(gsonData) | ||
activate ":FileWriter" | ||
":FileWriter" --> ":File" : String |
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 this should be a solid arrow since it's a method invocation?
docs/DeveloperGuide.md
Outdated
## Getting started | ||
|
||
Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md). | ||
|
||
|
||
## Design & implementation |
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.
Very clear explanation of the features! Would be helpful to have an explanation of why you implemented your feature logic this way so other developers can understand your considerations.
|
||
##### Save Events | ||
|
||
![Save To File Sequence Diagram](UML/Images/SaveToFile.png) |
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.
|
||
##### Load Events | ||
|
||
![Load Events Sequence Diagram](UML/Images/loadEvents.png) |
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.
Perhaps the explanation can be more in depth to allow for a better understanding of the function?
|
||
The class diagram below illustrates the structure of the EventList component. | ||
|
||
<img src="UML\Images\EventListUML.png" /> |
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 UML diagram below illustrates the flow of how the application adds modules: | ||
![Add Event Class Diagram](UML/Images/addModules.png) |
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.
Overall, the developer guide is easy to read and quite clear. However, some of the UML Diagrams can be further improved by following the conventions (e.g having an activation bar)
When the application starts up, the storage loadEvents() function will be called to load contents in the save file. | ||
|
||
##### Load Modules | ||
![Load Modules Sequence Diagram](UML/Images/LoadModules.png) |
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.
Clear and easy to read UML Diagram!
This makes life easier to developers in the future if they wish to add new features that requires users to use new commands. | ||
|
||
### Storage Component | ||
API: `Storage.java` |
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.
Well formatted and easy to read!
Parser -> Ui : addSuccessMsg() | ||
Ui --> Parser | ||
Parser --> Actor | ||
@enduml |
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.
Perhaps you include the activation bar using activate
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.
Additionally, remember to include the cross when you are no longer using the object/ it is deleted.
end | ||
end | ||
Parser --> Actor | ||
@enduml |
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.
Similarly as above, you can include an activation bar by using activate
docs/DeveloperGuide.md
Outdated
## Getting started | ||
|
||
Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md). | ||
|
||
|
||
## Design & implementation |
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.
Perhaps you could include more elaboration such has system architecture and dependencies as well as relevant diagrams for clarity! However, it is already very clear!
activate ":Storage" | ||
":Storage" -> ":EventListStorage" : loadEvents() | ||
activate ":EventListStorage" | ||
":EventListStorage" ->":EventListAdapter" : gson.fromJson(fileReader, ArrayList.class) |
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 this correct? I was under the assumption that when your arrow is drawn to a class, you are calling a method in that class. Is fromJson()
a method in :EventListAdapter
?
Implemented logger & added logging
docs/DeveloperGuide.md
Outdated
## Getting started | ||
|
||
Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md). | ||
|
||
|
||
## Design & implementation |
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 like how all the diagrams are in details and that's a whole lotta effort to do it! However it would be greater if you can have an overall architecture!
|
||
The Parser component parses the command of the user input and breaks the user input into different parts based on the flags. | ||
This component also ensures to validate that user input is correct. | ||
|
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.
|
||
When the application starts up, the storage loadEvents() function will be called to load contents in the save file. | ||
Similarly, the state of the user's event list is saved when the user exits the application by calling saveToFile(). | ||
|
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
|
||
And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted. | ||
|
||
<img src="UML\Images\EventListSD.png" style="zoom:80%;" /> |
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.
Add User Stories, Add Architecture documentation and dependencies.
Documentation
Added to DG
Bug fix storage
…into StorageUpgrade
added on to team-based tasks
Fix Bug in parseDeleteCommand
Fix bug for empty description or whitespace desription
add Xingyu's PPP
NUSplanner is a desktop app that allows for an easy and straightforward way for NUS students to manage their schedule ranging from person, school or external related activities. This application makes use of a desktop Command Line Interface (CLI), enabling a quick and sleek method of getting your schedule in check.