Merge pull request #188 from junhuplim/master

alright guys so this PR has settled these 3 issues.

undo/redo success message changed to specify what commands are being undone/redone
Add/Delete training Undo/Redo
Edit Person Undo/Redo
If im not wrong, I am left with dom's part and clear too.

docs/DeveloperGuide.adoc

@@ -396,85 +396,70 @@ down the list of people.
 ** Cons: There may be 2 people with the same name and thus result in an error.
 
 === Undo / Redo feature
-==== Proposed Implementation
+The undo command enables users to undo their previous commands while the redo feature enables users to redo their undone commands.
 
-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:
+==== Undo Implementation
 
-* `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.
+The undo feature is facilitated by the `HistoryManager`. The `commands` stack and the `addressBooks` stack in the `HistoryManager` class are used to govern the undo feature.
 
-These operations are exposed in the `Model` interface as `Model#commitAddressBook()`, `Model#undoAddressBook()` and `Model#redoAddressBook()` respectively.
+The static `commands` stack keeps track of all the commands that have been executed by the user while the static `addressBooks` stack keeps track of the corresponding state of the address book following the executed command.
 
-Given below is an example usage scenario and how the undo/redo mechanism behaves at each step.
+Given below is an example usage scenario and how the undo mechanism behaves at each step.
 
-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.
+Step 1. The user launches the application for the first time. The `HistoryManager` will be initialised with the
+initial address book state pushed to the `addressBooks` stack.
 
-image::UndoRedoState0.png[]
+image::initialStack.png[width=450]
 
-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.
+Step 2. The user executes the delete 3 command to delete the 3rd person in the Athletick list. The delete command will be pushed into the `commands` stack in and the new state of the address book (after executing the delete command) will be pushed to the `addressBooks` stack as well.
 
-image::UndoRedoState1.png[]
+image::afterUndoStack.png[width=450]
 
-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`.
+Step 3. The user now decides that deleting the 3rd person in the list was a mistake, and decides to undo the action by executing the `undo` command. The `undo` command calls the undo method in the `ModelManager` which pops the latest Command from the `commands` stack and
+the latest Address Book from the `addressBooks` stack. This removes the latest action caused by the command that wants to be undone. It then continues to peek at the `addressBooks` stack to retrieve the Address Book state before the undone Command was executed.
 
-image::UndoRedoState2.png[]
-
-[NOTE]
-If a command fails its execution, it will not call `Model#commitAddressBook()`, so the address book state will not be saved into the `addressBookStateList`.
-
-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::UndoRedoState3.png[]
-
-[NOTE]
-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.
+image::initialStack.png[width=450]
 
 The following sequence diagram shows how the undo operation works:
 
-image::UndoSequenceDiagram.png[]
+image::UndoSQ.png[width=450]
+Figure 1. Sequence diagram for undo feature
 
-NOTE: The lifeline for `UndoCommand` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram.
+==== Redo Implementation
 
-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.
+The redo feature is also facilitated by the `HistoryManager`. The `undoneCommands` stack and the `undoneAddressBooks` stack in the `HistoryManager` class are used to govern the redo feature.
 
-[NOTE]
-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 `undoneCommands` stack keeps track of commands that have been undone by the user while the `undoneAddressBooks` stack keeps track of the corresponding state of the address book following the undone command.
 
-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.
+How the redo feature works is very similar to how the undo feature works. The difference is that redo utilises the
+`undoneCommands` stack and the `undoneAddressBooks` stack. As such, you can also refer to the diagrams in the
+link:https://github.com/AY1920S1-CS2103T-T12-3/main/blob/master/docs/DeveloperGuide.adoc#undo-redo-feature[Undo
+Implementation.]
 
-image::UndoRedoState4.png[]
-
-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::UndoRedoState5.png[]
-
-The following activity diagram summarizes what happens when a user executes a new command:
-
-image::CommitActivityDiagram.png[]
 
 ==== Design Considerations
+This section describes the pros and cons of the current and other alternative implementations of the undo and redo features.
 
 ===== Aspect: How undo & redo executes
 
-* **Alternative 1 (current choice):** Saves the entire address book.
-** Pros: Easy to implement.
+* **Alternative 1 (current choice):** Saves a deep copy of the entire address book after each command into a stack.
+** Pros:  Easy to implement, and easy for developers to understand.
 ** Cons: May have performance issues in terms of memory usage.
 * **Alternative 2:** Individual command knows how to undo/redo by itself.
 ** 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.
+** Cons: We must ensure that the implementation of each individual command is correct.
+
+====== Reason why we chose alternative 1:
+Even though the memory usage of Alternative 2 is lesser, we do not feel this benefit of lesser memory usage
+outweighs the tedious cost of implementing the alternative.
 
-===== Aspect: Data structure to support the undo/redo commands
+Furthermore, as we realise that each time the application starts, the memory of the address books states
+are cleared.
+This means that the cost of having alternative 1 is significantly lesser, as the memory of the address
+book states does not accumulate. As such, we
+decided to
+go with the first alternative.
 
-* **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 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[]
 
 === Adding/editing photo feature
 ==== Implementation

docs/UserGuide.adoc

@@ -97,7 +97,7 @@ attendance and performance.
 
 Type the athlete's particulars in the format given below.
 
-Format : `add n/NAME p/PHONE e/EMAIL a/ADDRESS [t/TAG]... [i/IMAGE]`
+Format : `add n/NAME p/PHONE e/EMAIL g/GENDER a/ADDRESS [t/TAG]... [i/IMAGE]`
 
 [NOTE]
 ====
@@ -108,7 +108,7 @@ You can include any number of tags (zero tags are also allowed) to an athlete an
 is optional.
 ====
 
-Example: `add n/John Doe p/98765432 e/johnd@example.com a/311, Clementi Ave 2, #02-25 t/backstroke i/john.png`
+Example: `add n/John Doe p/98765432 e/johnd@example.com g/male a/311, Clementi Ave 2, #02-25 t/backstroke i/john.png`
 
 image::beforeAdd.png[width=450]
 
@@ -636,26 +636,28 @@ This command restores Athletick to the state before the previous command was exe
 [NOTE]
 ====
 Take note that the `undo` feature only applies to undoable commands.
-Undoable commands include: `add`, `delete`, `edit`, `clear`, `attendance` and `training`.
+Undoable commands include: `add`, `delete`, `edit`, `clear` and `training`.
 
 The `undo` command will not be able to undo commands that cannot be undone.
 Let’s say you have executed a `list` command to list out all the athletes information in Athletick.
 If you were to execute the `undo` command now, the `undo` command will fail because `list` is not an undoable command,
 and that no doable commands were executed before this.
 
 The `undo` command reverses previous commands in reverse chronological order.
-Let’s say you have executed the `edit` command, followed by the `delete` command.
-Executing `undo` now will first reverse the `delete` command.
-Executing `undo` again will reverse the `edit` command.
+Let’s say you have executed the `edit` command, followed by the `delete` command. If you were
+to execute `undo` command now, you will reverse the `delete` command. Executing `undo` again
+will then reverse the `edit` command.
 
 The `undo` command will reverse the latest command that can be undone.
 Let’s say you have executed the `delete` command, followed by the `list` command.
-Since `list` command is not an undoable command, executing `undo` now will thus reverse the `delete` command.
+Since `list` command is not an undoable command, if you were to execute `undo`
+command now, you will thus reverse the `delete` command.
 ====
 
 Let’s say you have accidentally deleted an athlete’s contact (Mohamad Ali) from your list.
 Instead of having to re-enter Mohamad Ali’s contact information all over again,
-you can easily restore all of Mohamad Ali’s details by `undo`-ing the `delete` command that you have just entered.
+you can easily restore all of Mohamad Ali’s details by executing the command `undo` to
+undo the `delete` command that you have just entered.
 
 *What you should do*
 
@@ -665,6 +667,7 @@ Format : `undo`
 
 image::undo.png[width=450]
 
+
 *What you should see*
 
 The result box will display the message “Undo Command Success” and you can check that Mohamad Ali’s
@@ -686,13 +689,17 @@ The `redo` command reverses previous `undo` commands in reverse chronological or
 Let’s say that you have executed the `clear` command, followed by the `add` command.
 Executing the `undo` command now will reverse the `add` command.
 Executing the `undo` command again will reverse the `clear` command as well.
-Following this, executing the `redo` command will reverse the last `undo` command and re-execute the `clear` command.
-Executing the `redo` command again will reverse the second-last `undo` command and re-execute the `add` command.
+Following this, executing the `redo` command will reverse the most recent `undo` command and re-execute the `clear`
+command.
+Executing the `redo` command again will reverse the second most recent `undo` command and re-execute the `add` command.
+
+The `redo` command can only be executed immediately after a `undo` command.
 ====
 
 Let’s say you have executed the `delete` command to delete Mohamad Ali from your list.
 You may undo this action and restore Mohamad Ali’s information by executing the `undo` command. (See Undoing a previous command.)
 
+
 Then, if you decide that you want the contact to remain deleted after all,
 you may very quickly do so by executing the `redo` command to reverse the `undo` command that you had just executed.
 
@@ -707,7 +714,7 @@ image::redo.png[width=450]
 
 The result box will display the message “Redo Command Success” and Mohamad Ali is once again gone from the list!
 
-image::afterUndo.png[width=450]
+image::afterRedo.png[width=450]
 
 === Upcoming features
 

src/main/java/seedu/address/MainApp.java

@@ -54,7 +54,6 @@ public class MainApp extends Application {
     protected Storage storage;
     protected Model model;
     protected Config config;
-
     @Override
     public void init() throws Exception {
         logger.info("=============================[ Initializing AddressBook ]===========================");
@@ -81,6 +80,7 @@ public void init() throws Exception {
 
         ui = new UiManager(logic, model);
         HistoryManager.getAddressBooks().push(model.getAthletickDeepCopy());
+        HistoryManager.getTrainingLists().push(model.getTrainingsDeepCopy(model.getAttendance().getTrainings()));
     }
 
     /**

src/main/java/seedu/address/logic/LogicManager.java

@@ -10,6 +10,9 @@
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
+import seedu.address.logic.commands.DeleteTrainingCommand;
+import seedu.address.logic.commands.EditCommand;
+import seedu.address.logic.commands.TrainingCommand;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.logic.parser.AthletickParser;
 import seedu.address.logic.parser.exceptions.ParseException;
@@ -44,9 +47,12 @@ public CommandResult execute(String commandText) throws CommandException, ParseE
         CommandResult commandResult;
         Command command = athletickParser.parseCommand(commandText);
         commandResult = command.execute(model);
+        if (command instanceof DeleteTrainingCommand || command instanceof TrainingCommand
+            || command instanceof EditCommand) {
+            HistoryManager.getTrainingLists().push(model.getTrainingsDeepCopy(model.getAttendance().getTrainings()));
+        }
         HistoryManager.getCommands().push(command);
         HistoryManager.getAddressBooks().push(model.getAthletickDeepCopy());
-
         try {
             storage.saveAthletick(model.getAthletick());
             storage.saveEvents(model.getPerformance());

src/main/java/seedu/address/logic/commands/AddCommand.java

@@ -74,4 +74,8 @@ public boolean equals(Object other) {
                 || (other instanceof AddCommand // instanceof handles nulls
                 && toAdd.equals(((AddCommand) other).toAdd));
     }
+    @Override
+    public String toString() {
+        return "Add '" + toAdd + "' Command";
+    }
 }

src/main/java/seedu/address/logic/commands/AttendanceCommand.java

@@ -74,4 +74,8 @@ public CommandResult execute(Model model) throws CommandException {
     public boolean isUndoable() {
         return false;
     }
+    @Override
+    public String toString() {
+        return "Attendance Command";
+    }
 }

src/main/java/seedu/address/logic/commands/CalendarCommand.java

@@ -58,4 +58,8 @@ public boolean equals(Object other) {
     public boolean isUndoable() {
         return false;
     }
+    @Override
+    public String toString() {
+        return "Calendar Command";
+    }
 }

src/main/java/seedu/address/logic/commands/ClearCommand.java

@@ -27,4 +27,8 @@ public CommandResult execute(Model model) {
     public boolean isUndoable() {
         return true;
     }
+    @Override
+    public String toString() {
+        return "Clear Command";
+    }
 }

src/main/java/seedu/address/logic/commands/DeleteCommand.java

@@ -20,5 +20,8 @@ public abstract class DeleteCommand extends Command {
     public boolean isUndoable() {
         return true;
     }
-
+    @Override
+    public String toString() {
+        return "Delete Command";
+    }
 }

src/main/java/seedu/address/logic/commands/DeletePersonCommand.java

@@ -23,7 +23,7 @@ public class DeletePersonCommand extends DeleteCommand {
     public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s";
 
     private final Index targetIndex;
-
+    private Person personToDelete;
     public DeletePersonCommand(Index targetIndex) {
         this.targetIndex = targetIndex;
     }
@@ -38,6 +38,7 @@ public CommandResult execute(Model model) throws CommandException {
         }
 
         Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
+        this.personToDelete = personToDelete;
         model.deletePerson(personToDelete);
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
@@ -48,4 +49,8 @@ public boolean equals(Object other) {
                 || (other instanceof DeletePersonCommand // instanceof handles nulls
                 && targetIndex.equals(((DeletePersonCommand) other).targetIndex)); // state check
     }
+    @Override
+    public String toString() {
+        return "Delete '" + personToDelete + "' Command";
+    }
 }

src/main/java/seedu/address/logic/commands/DeleteTrainingCommand.java

@@ -7,6 +7,7 @@
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.date.AthletickDate;
+import seedu.address.model.training.Training;
 
 /**
  * Deletes a training identified by the {@code date}.
@@ -24,15 +25,19 @@ public class DeleteTrainingCommand extends DeleteCommand {
     public static final String MESSAGE_NO_TRAINING_ON_DATE = "No training found on %s";
 
     private final AthletickDate date;
+    private Training trainingToBeDeleted;
 
     public DeleteTrainingCommand(AthletickDate date) {
         this.date = date;
     }
-
+    public AthletickDate getDate() {
+        return this.date;
+    }
     @Override
     public CommandResult execute(Model model) throws CommandException {
         if (model.hasTrainingOnDate(date)) {
-            model.deleteTrainingOnDate(date);
+            Training trainingToBeDeleted = model.deleteTrainingOnDate(date);
+            this.trainingToBeDeleted = trainingToBeDeleted;
         } else {
             throw new CommandException(String.format(MESSAGE_NO_TRAINING_ON_DATE, date));
         }
@@ -46,4 +51,8 @@ public boolean equals(Object other) {
                 || (other instanceof DeleteTrainingCommand // instanceof handles nulls
                 && date.equals(((DeleteTrainingCommand) other).date)); // state check
     }
+    @Override
+    public String toString() {
+        return "Delete '" + trainingToBeDeleted + "' Command";
+    }
 }