[CS2113-F13-3] NUSplanner by kyrixn · Pull Request #52 · nus-cs2113-AY2223S2/tp

kyrixn · 2023-03-03T05:29:31Z

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.

Resolve merge conflict in nus-cs2113-AY2223S2#51

quanganh2810 · 2023-03-29T05:31:57Z

+
+#### How the feature is implemented:
+
+![Storage Class Diagram](UML/Images/StorageSequenceDiagram.png)


The diagram is good but it looks quite hectic. Consider breaking it up using reference frames with proper annotation for easier comprehension

quanganh2810 · 2023-03-29T05:34:24Z

+
+The class diagram below illustrates the structure of the storage package
+
+![Storage Class Diagram](UML/Images/StorageClass.png)


Simple and easy to read !

quanganh2810 · 2023-03-29T05:41:05Z

+
+The class diagram below illustrates the structure of the EventList component.
+
+<img src="UML\Images\EventListUML.png" />


Perhaps you should consider noting down the methods within the UI and Parser classes

quanganh2810 · 2023-03-29T05:46:14Z

+
+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%;" />


Good job nice and easy to read !

Masahiro21

Overall the diagrams are very detailed and the explanations were easy to understand.

Masahiro21 · 2023-03-29T07:21:21Z

+<img src="UML\Images\EventListUML.png" />
+
+And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted.
+


Not a bug but the arrow is out of place compared to the rest of diagram, to improve on the quality of the diagram you can fix it.

Masahiro21 · 2023-03-29T07:28:33Z

+![Storage Class Diagram](UML/Images/StorageClass.png)
+
+#### How the feature is implemented:
+


<> if file does not exist should be encased in the alt box since it only runs under the condition that the file does not exist

Masahiro21 · 2023-03-29T07:31:11Z

+![Storage Class Diagram](UML/Images/StorageClass.png)
+
+#### How the feature is implemented:
+


For self-call methods should add another activation bar to prevent confusion

Masahiro21 · 2023-03-29T07:35:39Z

+<img src="UML\Images\EventListUML.png" />
+
+And below is a sequential diagram showing a event being added, revised, checked for information and finally deleted.
+


Just some spelling errors in the sequential diagram that can be fixed for better-quality work

haoyangw

Very concise and clear explanation of your features! Just some minor syntax issues with the UML diagrams. Great work! :)

haoyangw · 2023-03-30T04:46:24Z

@@ -0,0 +1,39 @@
+@startuml
+Actor -> Parser : parseAddCommand()


I think you should add a activate Parser line since the activation bar is missing from your diagram?

haoyangw · 2023-03-30T04:49:15Z

+-> ":Storage" : loadModules()
+activate ":Storage"
+Alt modulesLoaded
+    ":Storage" --> ":Storage" : :HashMap<String, NusModule>


I think this should be a solid arrow since it's a method invocation?

haoyangw · 2023-03-30T04:51:01Z

+    end
+    ":EventListStorage" -> ":FileWriter" : taskWriter.write(gsonData)
+    activate ":FileWriter"
+    ":FileWriter" --> ":File" : String


I think this should be a solid arrow since it's a method invocation?

haoyangw · 2023-03-30T04:53:28Z

+Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md).
+

 ## Design & implementation


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.

Reynold-SL · 2023-03-30T04:32:02Z

+
+##### Save Events
+
+![Save To File Sequence Diagram](UML/Images/SaveToFile.png)


Perhaps the activation bar can be a single one rather than split as the object is created once and called again later?

Reynold-SL · 2023-03-30T04:40:20Z

+
+##### Load Events
+
+![Load Events Sequence Diagram](UML/Images/loadEvents.png)


Perhaps the explanation can be more in depth to allow for a better understanding of the function?

Reynold-SL · 2023-03-30T04:44:55Z

+
+The class diagram below illustrates the structure of the EventList component.
+
+<img src="UML\Images\EventListUML.png" />


Perhaps the activation bar can be a single one as the object is created once and called again

Reynold-SL · 2023-03-30T04:49:57Z

+
+
+The UML diagram below illustrates the flow of how the application adds modules:
+![Add Event Class Diagram](UML/Images/addModules.png)


Perhaps you can include an activation bar as the object is created and for better visual understanding?

irving11119

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)

irving11119 · 2023-03-30T05:15:52Z

+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)


Clear and easy to read UML Diagram!

irving11119 · 2023-03-30T05:17:45Z

+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`


Well formatted and easy to read!

irving11119 · 2023-03-30T05:19:33Z

+Parser -> Ui : addSuccessMsg()
+Ui --> Parser
+Parser --> Actor
+@enduml


Perhaps you include the activation bar using activate

Additionally, remember to include the cross when you are no longer using the object/ it is deleted.

irving11119 · 2023-03-30T05:20:43Z

+    end
+end
+Parser --> Actor
+@enduml


Similarly as above, you can include an activation bar by using activate

irving11119 · 2023-03-30T05:31:40Z

+Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md).
+

 ## Design & implementation


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!

irving11119 · 2023-03-30T05:41:28Z

+activate ":Storage"
+":Storage" -> ":EventListStorage" : loadEvents()
+activate ":EventListStorage"
+":EventListStorage" ->":EventListAdapter" : gson.fromJson(fileReader, ArrayList.class)


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

alexgoexercise · 2023-03-30T07:27:49Z

+Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md).
+

 ## Design & implementation


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!

alexgoexercise · 2023-03-30T07:42:59Z

+
+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.
+


This sequence diagram is missing activation bars, it would be better if you include them

alexgoexercise · 2023-03-30T07:47:39Z

+
+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().
+


I would recommend using a constructor notation like what we have learnt in this module instead of "<>" notation for creating a constructor.

alexgoexercise · 2023-03-30T07:51:07Z

+
+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 is a red dot in the middle of the diagram, take note!

Add User Stories, Add Architecture documentation and dependencies.

Documentation

Added to DG

Bug fix storage

…into StorageUpgrade

Fix DG Bug

fix bug

Improved specificity

Formatting

Fix Bugs

Removed typo

added on to team-based tasks

Update DG

DG Fix

Fix Bug in parseDeleteCommand

Fix bug for empty description or whitespace desription

add Xingyu's PPP

kyrixn pushed a commit to kyrixn/tp that referenced this pull request Mar 16, 2023

Merge pull request nus-cs2113-AY2223S2#52 from Zemdalk/fix-edit-index

526caa5

Resolve merge conflict in nus-cs2113-AY2223S2#51

quanganh2810 reviewed Mar 29, 2023

View reviewed changes

Masahiro21 reviewed Mar 29, 2023

View reviewed changes

haoyangw reviewed Mar 30, 2023

View reviewed changes

Reynold-SL reviewed Mar 30, 2023

View reviewed changes

irving11119 reviewed Mar 30, 2023

View reviewed changes

hrithie and others added 2 commits March 30, 2023 15:08

Fixed checkstyle

b86f624

Merge pull request #94 from hrithie/hrithieMenon-v2.0

f2be2cd

Implemented logger & added logging

alexgoexercise reviewed Mar 30, 2023

View reviewed changes

kyrixn added 4 commits March 30, 2023 15:31

tmp

4999005

handle confliction

f8a8bae

Merge branch 'tt'

8054e60

add format check for -r

9b2e804

alexgoexercise reviewed Mar 30, 2023

View reviewed changes

matthew-liu-zhenjie and others added 16 commits March 30, 2023 16:28

Update UserGuide

fdedcb5

start revising UserUtility

c6ed908

Fix typo in UserGuide

c4a1298

Add Architecture class diagram

54bd7a2

Update Developer Guide

c27df2a

Add User Stories, Add Architecture documentation and dependencies.

Add NFR to Developer Guide

a49f77d

Merge pull request #95 from matthew-liu-zhenjie/Documentation

360a8ac

Documentation

Added to DG

1918260

Fix bug where .jar does not load nusmods data

6590b1b

Remove debugging println

d7fdab9

Add JUnit test for StorageManager

0aa3573

Update EventListAdapter

49d8aa5

Merge pull request #96 from hrithie/hrithieMenon-v2.0

eff9915

Added to DG

Merge pull request #97 from matthew-liu-zhenjie/BugFixStorage

1b5cc38

Bug fix storage

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

76e48bf

…into StorageUpgrade

writing recurring evvent in tt, haven't finished

a3885ca

kyrixn and others added 30 commits April 10, 2023 22:01

fix bugs in delete

53bf4cf

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

4f1ed59

Merge pull request #181 from matthew-liu-zhenjie/master

1c82ebd

Fix DG Bug

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

10a17cf

Merge pull request #182 from kyrixn/master

ae2dc6d

fix bug

Update DG

bc4ee1a

Improved specificity

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

4cf784b

Update DG

4c8b288

Formatting

Update DG

96b743e

Fix Bugs

Fix DG

c169467

Removed typo

Fix bugs in DG

5fdd245

Improve DG

7a2ca7d

Fix typo in DG, update PPP

1e713f1

Add PPP

168c4df

Update PPP

0f4389d

Update PPP

eb3260f

added on to team-based tasks

DG Fix

ae94fe0

Merge pull request #183 from matthew-liu-zhenjie/master

ca9c97f

Update DG

Fix Bug in parseDeleteCommand

238e8bc

Merge pull request #184 from hrithie/hrithieMenon-v2.1

12d8089

DG Fix

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

9d88701

Merge pull request #185 from matthew-liu-zhenjie/master

9908e0f

Fix Bug in parseDeleteCommand

Fix bug for empty description or whitespace desription

6ee537c

Merge pull request #186 from matthew-liu-zhenjie/master

0715981

Fix bug for empty description or whitespace desription

add ppp

743edf3

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

d9ced7e

Merge pull request #188 from Zemdalk/master

59df5b3

add Xingyu's PPP

add assertions

7e0e8c8

Merge branch 'master' of https://github.com/AY2223S2-CS2113-F13-3/tp

d1f8e0b

Merge pull request #189 from matthew-liu-zhenjie/master

a77443f


		#### How the feature is implemented:

		![Storage Class Diagram](UML/Images/StorageSequenceDiagram.png)


		The class diagram below illustrates the structure of the storage package

		![Storage Class Diagram](UML/Images/StorageClass.png)


		The class diagram below illustrates the structure of the EventList component.

		<img src="UML\Images\EventListUML.png" />


		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%;" />

		@@ -0,0 +1,39 @@
		@startuml
		Actor -> Parser : parseAddCommand()

		Refer to the user guide [here](https://github.com/AY2223S2-CS2113-F13-3/tp/blob/master/docs/UserGuide.md).


		## Design & implementation


		##### Save Events

		![Save To File Sequence Diagram](UML/Images/SaveToFile.png)


		##### Load Events

		![Load Events Sequence Diagram](UML/Images/loadEvents.png)



		The UML diagram below illustrates the flow of how the application adds modules:
		![Add Event Class Diagram](UML/Images/addModules.png)


		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.


		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().

Conversation

kyrixn commented Mar 3, 2023

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!

Masahiro21 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!

haoyangw 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!

Choose a reason for hiding this comment

Uh oh!

irving11119 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants