diff --git a/docs/DeveloperGuide.md b/docs/DeveloperGuide.md index cf4ecd6863b..b8c0a07018f 100644 --- a/docs/DeveloperGuide.md +++ b/docs/DeveloperGuide.md @@ -154,6 +154,44 @@ Classes used by multiple components are in the `seedu.addressbook.commons` packa This section describes some noteworthy details on how certain features are implemented. +### Restricting Commands based on GUI state + +#### Rationale + +Some commands, e.g. `delete lesson` and `edit exam` should not be used in the view where the `Modules` are listed. +This is because the `delete` and `edit` commands are indexed based, and in the `Modules` view, there are multiple `Lesson`s or `Exam`s displayed with the same index. +Hence, there is a need to restrict these commands to the Module Details view, so that there is no ambiguity about which `Lesson` or `Exam` to delete. + +Thus, before commands are executed, the GUI state of the application needs to be checked to see if it is valid. + +#### Implementation + +To implement this feature, the GUI needs to keep track of which state it is in, so a field in `MainWindow` was created to store the `GuiState`. +In addition, the `Logic#execute()` method has an additional `GuiState` parameter, which will be the current `GuiState` of the `MainWindow`, and the `Parser` interface was modified so that `parse` also takes in a `GuiState` parameter. +After the command is executed, the `GuiState` of the `MainWindow` is updated with the `GuiState` of the returned `CommandResult`. + +The class diagram of how UI and Logic interact with each other is shown below. + +![LogicGuiStateClassDigram](images/LogicGuiStateClassDiagram.png) + +The sequence diagram of how this works for a `delete` command is found below. The objects in `Model` and `Storage` are not shown for simplicity. + +![GuiStateSequenceDiagram](images/GuiStateSEquenceDiagram.png) + +When parsing, the respective `Parser` will check the current `GuiState` with the allowed `GuiState`s. If the `GuiState` is valid, it will proceed with parsing the command. +Otherwise, it will throw a `GuiStateException`. + +#### Design considerations: + +**Aspect: How to implement edit/delete lessons/exams:** + +* **Alternative 1 (current choice)**: Limits these two commands to the Module Details view. + * Pros: Easier to implement. + * Cons: Requires passing `GuiState` from the `UI` component to the `Logic` component, reducing cohesion. +* **Alternative 2**: Combine the lists of lessons and exams into one central list in the List Lessons or Exams view respectively. + * Pros: Does not require `MainWindow` to keep track of its current `GuiState`. + * Cons: Difficult to implement - have to figure out how to map the `Lesson` or `Exam` from the central list to its original module. + ### \[Proposed\] Undo/redo feature #### Proposed Implementation diff --git a/docs/diagrams/GuiStateSequenceDiagram.puml b/docs/diagrams/GuiStateSequenceDiagram.puml new file mode 100644 index 00000000000..3d3951e399b --- /dev/null +++ b/docs/diagrams/GuiStateSequenceDiagram.puml @@ -0,0 +1,59 @@ +@startuml +!include style.puml + +box UI UI_COLOR_T1 +participant ":MainWindow" as MainWindow UI_COLOR +end box + +box Logic LOGIC_COLOR_T1 +participant ":LogicManager" as LogicManager LOGIC_COLOR +participant ":AddressBookParser" as AddressBookParser LOGIC_COLOR +participant ":DeleteCommandParser" as DeleteCommandParser LOGIC_COLOR +participant "d:DeleteCommand" as DeleteCommand LOGIC_COLOR +participant "r:CommandResult" as CommandResult LOGIC_COLOR +end box + +[-> MainWindow : executeCommand(delete 1) +activate MainWindow + +MainWindow -> LogicManager : execute(delete 1, GuiState.DETAILS) +activate LogicManager + +LogicManager -> AddressBookParser : parseCommand(delete 1, GuiState.DETAILS) +activate AddressBookParser + +AddressBookParser -> DeleteCommandParser : parse(1, GuiState.DETAILS) +activate DeleteCommandParser + +create DeleteCommand +DeleteCommandParser -> DeleteCommand +activate DeleteCommand + +DeleteCommand --> DeleteCommandParser +deactivate DeleteCommand + +DeleteCommandParser --> AddressBookParser : d +deactivate DeleteCommandParser + +AddressBookParser --> LogicManager : d +deactivate AddressBookParser + +LogicManager -> DeleteCommand : execute() +activate DeleteCommand + +create CommandResult +DeleteCommand -> CommandResult +activate CommandResult + +CommandResult --> DeleteCommand +deactivate CommandResult + +DeleteCommand --> LogicManager : r +deactivate DeleteCommand + +LogicManager --> MainWindow : r +deactivate LogicManager + +[<--MainWindow +deactivate MainWindow +@enduml diff --git a/docs/diagrams/LogicGuiStateClassDiagram.puml b/docs/diagrams/LogicGuiStateClassDiagram.puml new file mode 100644 index 00000000000..b9d38f4a2ec --- /dev/null +++ b/docs/diagrams/LogicGuiStateClassDiagram.puml @@ -0,0 +1,62 @@ +@startuml +!include style.puml +skinparam arrowThickness 1.1 +skinparam arrowColor LOGIC_COLOR_T4 +skinparam classBackgroundColor LOGIC_COLOR + +package UI { + +Class MainWindow UI_COLOR +} + +package Logic { + +Class AddressBookParser +Class XYZCommand +Class XYZParser +Class CommandResult + +Class "{abstract}\nCommand" as Command + +Class "<>\nGuiState" as GuiState +Interface Logic <> +Interface Parser <> + +Class LogicManager + +} + +package Model{ +Class HiddenModel #FFFFFF +} + +package Storage{ +} + +MainWindow .> Logic + +LogicManager .right|> Logic +MainWindow ->"1" GuiState +Logic .right> GuiState + +XYZCommand -up-|> Command +LogicManager ..> Command : executes > + +LogicManager -->"1" AddressBookParser +AddressBookParser ..> Parser +XYZParser .up.|> Parser +XYZParser .down.> XYZCommand : creates > + +LogicManager --> Model +LogicManager --> Storage +Storage --[hidden] Model +Command .[hidden]up.> Storage +Command .left.> Model +note left of XYZCommand: XYZCommand = AddCommand, \nDeleteCommand, etc + +Logic ..> CommandResult +LogicManager .down.> CommandResult +Command .up.> CommandResult : produces > +CommandResult -up->"1" GuiState +MainWindow .down.> CommandResult : updates < +@enduml diff --git a/docs/images/GuiStateSequenceDiagram.png b/docs/images/GuiStateSequenceDiagram.png new file mode 100644 index 00000000000..643fb848ffa Binary files /dev/null and b/docs/images/GuiStateSequenceDiagram.png differ diff --git a/docs/images/LogicGuiStateClassDiagram.png b/docs/images/LogicGuiStateClassDiagram.png new file mode 100644 index 00000000000..2c53400d8aa Binary files /dev/null and b/docs/images/LogicGuiStateClassDiagram.png differ