DeveloperGuide: update section on UndoRedoStack to VersionedAddressBook

The app previously uses UndoRedoStack which stores the UndoableCommands in LogicManager for the undo/redo mechanism. Since the undo/redo machanism has been shifted to VersionedAddressBook which stores ReadOnlyAddressBook in a List, and has been moved to Model layer instead, the sections on undo/redo feature, Logic and Model component in DeveloperGuide is out of date. Also, commands that do not mutate the address book, such as FindCommand and ListCommand, will no longer clear the redoable address book states. This allows for redo commands to pass when we undo commands and execute a non-mutating command afterwards. UndoableCommand has also been removed from the codebase as well. Let's update the out of date sections in the DeveloperGuide, such as switching from UndoRedoStack to the updated VersionedAddressBook, how the undo/redo mechanism works, and remove any reference to UndoableCommand in DeveloperGuide.
AY2324S2-CS2103-F09-2 · Jul 6, 2018 · d946766 · d946766
1 parent 8fb3f9a
commit d946766
Show file tree

Hide file tree

Showing 35 changed files with 32 additions and 83 deletions.
diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
@@ -184,9 +184,6 @@ The `UI` component,
 .Structure of the Logic Component
 image::LogicClassDiagram.png[width="800"]
 
-.Structure of Commands in the Logic Component. This diagram shows finer details concerning `XYZCommand` and `Command` in <<fig-LogicClassDiagram>>
-image::LogicCommandClassDiagram.png[width="800"]
-
 *API* :
 link:{repoURL}/src/main/java/seedu/address/logic/Logic.java[`Logic.java`]
 
@@ -246,99 +243,63 @@ This section describes some noteworthy details on how certain features are imple
 === Undo/Redo feature
 ==== Current Implementation
 
-The undo/redo mechanism is facilitated by an `UndoRedoStack`, which resides inside `LogicManager`. It supports undoing and redoing of commands that modifies the state of the address book (e.g. `add`, `edit`). Such commands will inherit from `UndoableCommand`.
-
-`UndoRedoStack` only deals with `UndoableCommands`. Commands that cannot be undone will inherit from `Command` instead. The following diagram shows the inheritance diagram for commands:
-
-image::LogicCommandClassDiagram.png[width="800"]
+The undo/redo mechanism is facilitated by `VersionedAddressBook`.
+It extends `AddressBook` with an undo/redo history, stored internally as an `addressBookStateList` and `currentStatePointer`.
+Additionally, it implements the following operations:
 
-As you can see from the diagram, `UndoableCommand` adds an extra layer between the abstract `Command` class and concrete commands that can be undone, such as the `DeleteCommand`. Note that extra tasks need to be done when executing a command in an _undoable_ way, such as saving the state of the address book before execution. `UndoableCommand` contains the high-level algorithm for those extra tasks while the child classes implements the details of how to execute the specific command. Note that this technique of putting the high-level algorithm in the parent class and lower-level steps of the algorithm in child classes is also known as the https://www.tutorialspoint.com/design_pattern/template_pattern.htm[template pattern].
+* `VersionedAddressBook#commit()` -- Saves the current address book state in its history.
+* `VersionedAddressBook#undo()` -- Restores the previous address book state from its history.
+* `VersionedAddressBook#redo()` -- Restores a previously undone address book state from its history.
 
-Commands that are not undoable are implemented this way:
-[source,java]
-----
-public class ListCommand extends Command {
-    @Override
-    public CommandResult execute() {
-        // ... list logic ...
-    }
-}
-----
+These operations are exposed in the `Model` interface as `Model#commitAddressBook()`, `Model#undoAddressBook()` and `Model#redoAddressBook()` respectively.
 
-With the extra layer, the commands that are undoable are implemented this way:
-[source,java]
-----
-public abstract class UndoableCommand extends Command {
-    @Override
-    public CommandResult execute() {
-        // ... undo logic ...
+Given below is an example usage scenario and how the undo/redo mechanism behaves at each step.
 
-        executeUndoableCommand();
-    }
-}
+Step 1. The user launches the application for the first time. The `VersionedAddressBook` will be initialized with the initial address book state, and the `currentStatePointer` pointing to that single address book state.
 
-public class DeleteCommand extends UndoableCommand {
-    @Override
-    public CommandResult executeUndoableCommand() {
-        // ... delete logic ...
-    }
-}
-----
+image::UndoRedoStartingStateListDiagram.png[width="800"]
 
-Suppose that the user has just launched the application. The `UndoRedoStack` will be empty at the beginning.
+Step 2. The user executes `delete 5` command to delete the 5th person in the address book. The `delete` command calls `Model#commitAddressBook()`, causing the modified state of the address book after the `delete 5` command executes to be saved in the `addressBookStateList`, and the `currentStatePointer` is shifted to the newly inserted address book state.
 
-The user executes a new `UndoableCommand`, `delete 5`, to delete the 5th person in the address book. The current state of the address book is saved before the `delete 5` command executes. The `delete 5` command will then be pushed onto the `undoStack` (the current state is saved together with the command).
+image::UndoRedoNewCommand1StateListDiagram.png[width="800"]
 
-image::UndoRedoStartingStackDiagram.png[width="800"]
+Step 3. The user executes `add n/David ...` to add a new person. The `add` command also calls `Model#commitAddressBook()`, causing another modified address book state to be saved into the `addressBookStateList`.
 
-As the user continues to use the program, more commands are added into the `undoStack`. For example, the user may execute `add n/David ...` to add a new person.
-
-image::UndoRedoNewCommand1StackDiagram.png[width="800"]
+image::UndoRedoNewCommand2StateListDiagram.png[width="800"]
 
 [NOTE]
-If a command fails its execution, it will not be pushed to the `UndoRedoStack` at all.
-
-The user now decides that adding the person was a mistake, and decides to undo that action using `undo`.
+If a command fails its execution, it will not call `Model#commitAddressBook()`, so the address book state will not be saved into the `addressBookStateList`.
 
-We will pop the most recent command out of the `undoStack` and push it back to the `redoStack`. We will restore the address book to the state before the `add` command executed.
+Step 4. The user now decides that adding the person was a mistake, and decides to undo that action by executing the `undo` command. The `undo` command will call `Model#undoAddressBook()`, which will shift the `currentStatePointer` once to the left, pointing it to the previous address book state, and restores the address book to that state.
 
-image::UndoRedoExecuteUndoStackDiagram.png[width="800"]
+image::UndoRedoExecuteUndoStateListDiagram.png[width="800"]
 
 [NOTE]
-If the `undoStack` is empty, then there are no other commands left to be undone, and an `Exception` will be thrown when popping the `undoStack`.
+If the `currentStatePointer` is at index 0, pointing to the initial address book state, then there are no previous address book states to restore. The `undo` command uses `Model#canUndoAddressBook()` to check if this is the case. If so, it will return an error to the user rather than attempting to perform the undo.
 
 The following sequence diagram shows how the undo operation works:
 
 image::UndoRedoSequenceDiagram.png[width="800"]
 
-The redo does the exact opposite (pops from `redoStack`, push to `undoStack`, and restores the address book to the state after the command is executed).
+The `redo` command does the opposite -- it calls `Model#redoAddressBook()`, which shifts the `currentStatePointer` once to the right, pointing to the previously undone state, and restores the address book to that state.
 
 [NOTE]
-If the `redoStack` is empty, then there are no other commands left to be redone, and an `Exception` will be thrown when popping the `redoStack`.
+If the `currentStatePointer` is at index `addressBookStateList.size() - 1`, pointing to the latest address book state, then there are no undone address book states to restore. The `redo` command uses `Model#canRedoAddressBook()` to check if this is the case. If so, it will return an error to the user rather than attempting to perform the redo.
 
-The user now decides to execute a new command, `clear`. As before, `clear` will be pushed into the `undoStack`. This time the `redoStack` is no longer empty. It will be purged as it no longer make sense to redo the `add n/David` command (this is the behavior that most modern desktop applications follow).
+Step 5. The user then decides to execute the command `list`. Commands that do not modify the address book, such as `list`, will usually not call `Model#commitAddressBook()`, `Model#undoAddressBook()` or `Model#redoAddressBook()`. Thus, the `addressBookStateList` remains unchanged.
 
-image::UndoRedoNewCommand2StackDiagram.png[width="800"]
+image::UndoRedoNewCommand3StateListDiagram.png[width="800"]
 
-Commands that are not undoable are not added into the `undoStack`. For example, `list`, which inherits from `Command` rather than `UndoableCommand`, will not be added after execution:
+Step 6. The user executes `clear`, which calls `Model#commitAddressBook()`. Since the `currentStatePointer` is not pointing at the end of the `addressBookStateList`, all address book states after the `currentStatePointer` will be purged. We designed it this way because it no longer makes sense to redo the `add n/David ...` command. This is the behavior that most modern desktop applications follow.
 
-image::UndoRedoNewCommand3StackDiagram.png[width="800"]
+image::UndoRedoNewCommand4StateListDiagram.png[width="800"]
 
-The following activity diagram summarize what happens inside the `UndoRedoStack` when a user executes a new command:
+The following activity diagram summarizes what happens when a user executes a new command:
 
 image::UndoRedoActivityDiagram.png[width="650"]
 
 ==== Design Considerations
 
-===== Aspect: Implementation of `UndoableCommand`
-
-* **Alternative 1 (current choice):** Add a new abstract method `executeUndoableCommand()`
-** Pros: We will not lose any undone/redone functionality as it is now part of the default behaviour. Classes that deal with `Command` do not have to know that `executeUndoableCommand()` exist.
-** Cons: Hard for new developers to understand the template pattern.
-* **Alternative 2:** Just override `execute()`
-** Pros: Does not involve the template pattern, easier for new developers to understand.
-** Cons: Classes that inherit from `UndoableCommand` must remember to call `super.execute()`, or lose the ability to undo/redo.
-
 ===== Aspect: How undo & redo executes
 
 * **Alternative 1 (current choice):** Saves the entire address book.
@@ -348,26 +309,14 @@ image::UndoRedoActivityDiagram.png[width="650"]
 ** Pros: Will use less memory (e.g. for `delete`, just save the person being deleted).
 ** Cons: We must ensure that the implementation of each individual command are correct.
 
-
-===== Aspect: Type of commands that can be undone/redone
-
-* **Alternative 1 (current choice):** Only include commands that modifies the address book (`add`, `clear`, `edit`).
-** Pros: We only revert changes that are hard to change back (the view can easily be re-modified as no data are * lost).
-** Cons: User might think that undo also applies when the list is modified (undoing filtering for example), * only to realize that it does not do that, after executing `undo`.
-* **Alternative 2:** Include all commands.
-** Pros: Might be more intuitive for the user.
-** Cons: User have no way of skipping such commands if he or she just want to reset the state of the address * book and not the view.
-**Additional Info:** See our discussion  https://github.com/se-edu/addressbook-level4/issues/390#issuecomment-298936672[here].
-
-
 ===== Aspect: Data structure to support the undo/redo commands
 
-* **Alternative 1 (current choice):** Use separate stack for undo and redo
-** Pros: Easy to understand for new Computer Science student undergraduates to understand, who are likely to be * the new incoming developers of our project.
-** Cons: Logic is duplicated twice. For example, when a new command is executed, we must remember to update * both `HistoryManager` and `UndoRedoStack`.
+* **Alternative 1 (current choice):** Use a list to store the history of address book states.
+** Pros: Easy for new Computer Science student undergraduates to understand, who are likely to be the new incoming developers of our project.
+** Cons: Logic is duplicated twice. For example, when a new command is executed, we must remember to update both `HistoryManager` and `VersionedAddressBook`.
 * **Alternative 2:** Use `HistoryManager` for undo/redo
-** Pros: We do not need to maintain a separate stack, and just reuse what is already in the codebase.
-** Cons: Requires dealing with commands that have already been undone: We must remember to skip these commands. Violates Single Responsibility Principle and Separation of Concerns as `HistoryManager` now needs to do two * different things.
+** Pros: We do not need to maintain a separate list, and just reuse what is already in the codebase.
+** Cons: Requires dealing with commands that have already been undone: We must remember to skip these commands. Violates Single Responsibility Principle and Separation of Concerns as `HistoryManager` now needs to do two different things.
 // end::undoredo[]
 
 // tag::dataencryption[]
@@ -759,12 +708,12 @@ Let's start by teaching the application how to parse a `remark` command. We will
 
 **Main:**
 
-. Add a `RemarkCommand` that extends link:{repoURL}/src/main/java/seedu/address/logic/commands/UndoableCommand.java[`UndoableCommand`]. Upon execution, it should just throw an `Exception`.
+. Add a `RemarkCommand` that extends link:{repoURL}/src/main/java/seedu/address/logic/commands/Command.java[`Command`]. Upon execution, it should just throw an `Exception`.
 . Modify link:{repoURL}/src/main/java/seedu/address/logic/parser/AddressBookParser.java[`AddressBookParser`] to accept a `RemarkCommand`.
 
 **Tests:**
 
-. Add `RemarkCommandTest` that tests that `executeUndoableCommand()` throws an Exception.
+. Add `RemarkCommandTest` that tests that `execute()` throws an Exception.
 . Add new test method to link:{repoURL}/src/test/java/seedu/address/logic/parser/AddressBookParserTest.java[`AddressBookParserTest`], which tests that typing "remark" returns an instance of `RemarkCommand`.
 
 ===== [Step 2] Logic: Teach the app to accept 'remark' arguments

diff --git a/docs/diagrams/LogicComponentClassDiagram.pptx b/docs/diagrams/LogicComponentClassDiagram.pptx
diff --git a/docs/diagrams/LogicComponentCommandClassDiagram.pptx b/docs/diagrams/LogicComponentCommandClassDiagram.pptx
diff --git a/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx b/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx
diff --git a/docs/diagrams/ModelComponentClassDiagram.pptx b/docs/diagrams/ModelComponentClassDiagram.pptx
diff --git a/docs/diagrams/UndoRedoActivityDiagram.pptx b/docs/diagrams/UndoRedoActivityDiagram.pptx
diff --git a/docs/diagrams/UndoRedoExecuteUndoStackDiagram.pptx b/docs/diagrams/UndoRedoExecuteUndoStackDiagram.pptx
diff --git a/docs/diagrams/UndoRedoExecuteUndoStateListDiagram.pptx b/docs/diagrams/UndoRedoExecuteUndoStateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand1StackDiagram.pptx b/docs/diagrams/UndoRedoNewCommand1StackDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand1StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand1StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand2StackDiagram.pptx b/docs/diagrams/UndoRedoNewCommand2StackDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand2StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand2StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand3StackDiagram.pptx b/docs/diagrams/UndoRedoNewCommand3StackDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand3StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand3StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand4StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand4StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoSequenceDiagram.pptx b/docs/diagrams/UndoRedoSequenceDiagram.pptx
diff --git a/docs/diagrams/UndoRedoStartingStackDiagram.pptx b/docs/diagrams/UndoRedoStartingStackDiagram.pptx
diff --git a/docs/diagrams/UndoRedoStartingStateListDiagram.pptx b/docs/diagrams/UndoRedoStartingStateListDiagram.pptx
diff --git a/docs/images/LogicClassDiagram.png b/docs/images/LogicClassDiagram.png
diff --git a/docs/images/LogicCommandClassDiagram.png b/docs/images/LogicCommandClassDiagram.png
diff --git a/docs/images/ModelClassBetterOopDiagram.png b/docs/images/ModelClassBetterOopDiagram.png
diff --git a/docs/images/ModelClassDiagram.png b/docs/images/ModelClassDiagram.png
diff --git a/docs/images/UndoRedoActivityDiagram.png b/docs/images/UndoRedoActivityDiagram.png
diff --git a/docs/images/UndoRedoExecuteUndoStackDiagram.png b/docs/images/UndoRedoExecuteUndoStackDiagram.png
diff --git a/docs/images/UndoRedoExecuteUndoStateListDiagram.png b/docs/images/UndoRedoExecuteUndoStateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand1StackDiagram.png b/docs/images/UndoRedoNewCommand1StackDiagram.png
diff --git a/docs/images/UndoRedoNewCommand1StateListDiagram.png b/docs/images/UndoRedoNewCommand1StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand2StackDiagram.png b/docs/images/UndoRedoNewCommand2StackDiagram.png
diff --git a/docs/images/UndoRedoNewCommand2StateListDiagram.png b/docs/images/UndoRedoNewCommand2StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand3StackDiagram.png b/docs/images/UndoRedoNewCommand3StackDiagram.png
diff --git a/docs/images/UndoRedoNewCommand3StateListDiagram.png b/docs/images/UndoRedoNewCommand3StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand4StateListDiagram.png b/docs/images/UndoRedoNewCommand4StateListDiagram.png
diff --git a/docs/images/UndoRedoSequenceDiagram.png b/docs/images/UndoRedoSequenceDiagram.png
diff --git a/docs/images/UndoRedoStartingStackDiagram.png b/docs/images/UndoRedoStartingStackDiagram.png
diff --git a/docs/images/UndoRedoStartingStateListDiagram.png b/docs/images/UndoRedoStartingStateListDiagram.png