Merge pull request nus-cs2103-AY2122S1#97 from luminousleek/update-dg

Update DG for design considerations

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

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

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 "<<enumeration>>\nGuiState" as GuiState
+Interface Logic <<Interface>>
+Interface Parser <<Interface>>
+
+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