Merge pull request #180 from jeffsieu/branch-Refactor-Undoable

Refactor Command to UndoableCommand, fix GUI not being undoable

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

@@ -5,16 +5,15 @@
 
 import javafx.collections.ObservableList;
 import seedu.address.commons.core.GuiSettings;
+import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.guiactions.GuiAction;
 import seedu.address.logic.parser.exceptions.ParseException;
 import seedu.address.model.ReadOnlyAddressBook;
 import seedu.address.model.ReadOnlyTaskList;
 import seedu.address.model.person.Person;
 import seedu.address.model.task.Task;
 import seedu.address.model.task.filters.TaskFilter;
-import seedu.address.ui.exceptions.GuiException;
 
 /**
  * API of the Logic component
@@ -30,10 +29,12 @@ public interface Logic {
     CommandResult execute(String commandText) throws CommandException, ParseException;
 
     /**
-     * Executes the GUI action.
-     * @param action The GUI action to execute
+     * Executes the command and returns the result.
+     * @param command The command to execute.
+     * @return the result of the command execution.
+     * @throws CommandException If an error occurs during command execution.
      */
-    void executeGuiAction(GuiAction action) throws GuiException;
+    CommandResult executeCommand(Command command) throws CommandException;
 
     /**
      * Adds a task filter to the list of selected filters.

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

@@ -10,8 +10,8 @@
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.CommandResult;
+import seedu.address.logic.commands.UndoableCommand;
 import seedu.address.logic.commands.exceptions.CommandException;
-import seedu.address.logic.guiactions.GuiAction;
 import seedu.address.logic.parser.AddressBookParser;
 import seedu.address.logic.parser.exceptions.ParseException;
 import seedu.address.model.Model;
@@ -21,7 +21,6 @@
 import seedu.address.model.task.Task;
 import seedu.address.model.task.filters.TaskFilter;
 import seedu.address.storage.Storage;
-import seedu.address.ui.exceptions.GuiException;
 
 /**
  * The main LogicManager of the app.
@@ -46,33 +45,28 @@ public LogicManager(Model model, Storage storage) {
     @Override
     public CommandResult execute(String commandText) throws CommandException, ParseException {
         logger.info("----------------[USER COMMAND][" + commandText + "]");
-
-        CommandResult commandResult;
         Command command = addressBookParser.parse(commandText);
-        commandResult = command.execute(model);
-        model.getCommandHistory().pushCommand(command);
+        // If successfully parsed, add to history
         model.addCommandToHistory(commandText);
-
-        try {
-            storage.saveAddressBook(model.getAddressBook());
-            storage.saveTaskList(model.getTaskList());
-        } catch (IOException ioe) {
-            throw new CommandException(FILE_OPS_ERROR_MESSAGE + ioe, ioe);
-        }
-
-        return commandResult;
+        return executeCommand(command);
     }
 
     @Override
-    public void executeGuiAction(GuiAction action) throws GuiException {
-        model.executeGuiAction(action);
+    public CommandResult executeCommand(Command command) throws CommandException {
+        CommandResult commandResult = command.execute(model);
+
+        if (command instanceof UndoableCommand) {
+            model.getCommandHistory().pushCommand((UndoableCommand) command);
+        }
 
         try {
             storage.saveAddressBook(model.getAddressBook());
             storage.saveTaskList(model.getTaskList());
         } catch (IOException ioe) {
-            throw new GuiException(FILE_OPS_ERROR_MESSAGE + ioe, ioe);
+            throw new CommandException(FILE_OPS_ERROR_MESSAGE + ioe, ioe);
         }
+
+        return commandResult;
     }
 
     @Override

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

@@ -18,7 +18,7 @@
 /**
  * Adds a person to the address book.
  */
-public class AddCommand extends Command {
+public class AddCommand extends UndoableCommand {
 
     public static final String COMMAND_WORD = "add";
 
@@ -53,7 +53,7 @@ public AddCommand(Person person) {
     }
 
     @Override
-    public CommandResult execute(Model model) throws CommandException {
+    protected CommandResult executeDo(Model model) throws CommandException {
         requireNonNull(model);
 
         if (model.hasPerson(toAdd)) {
@@ -64,16 +64,16 @@ public CommandResult execute(Model model) throws CommandException {
         return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
     }
 
+    @Override
+    protected CommandResult executeUndo(Model model) {
+        model.deletePerson(this.toAdd);
+        return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
+    }
+
     @Override
     public boolean equals(Object other) {
         return other == this // short circuit if same object
                 || (other instanceof AddCommand // instanceof handles nulls
                 && toAdd.equals(((AddCommand) other).toAdd));
     }
-
-    @Override
-    public CommandResult undo(Model model) throws CommandException {
-        model.deletePerson(this.toAdd);
-        return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
-    }
 }

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

@@ -2,8 +2,6 @@
 
 import static java.util.Objects.requireNonNull;
 
-import seedu.address.commons.core.Messages;
-import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.logic.parser.CommandSpecification;
 import seedu.address.model.AddressBook;
 import seedu.address.model.Model;
@@ -12,7 +10,7 @@
 /**
  * Clears the address book.
  */
-public class ClearCommand extends Command {
+public class ClearCommand extends UndoableCommand {
 
     public static final String COMMAND_WORD = "clear";
     public static final String MESSAGE_SUCCESS = "Address book has been cleared!";
@@ -25,23 +23,18 @@ public class ClearCommand extends Command {
     private ReadOnlyAddressBook oldAddressBook;
 
     @Override
-    public CommandResult execute(Model model) throws CommandException {
-        if (!this.canExecute) {
-            throw new CommandException(Messages.MESSAGE_UNABLE_TO_EXECUTE);
-        }
+    protected CommandResult executeDo(Model model) {
         requireNonNull(model);
 
         // Save a copy of the previous AddressBook
         this.oldAddressBook = new AddressBook(model.getAddressBook());
 
         model.setAddressBook(new AddressBook());
-        this.canExecute = false;
         return new CommandResult(MESSAGE_SUCCESS);
     }
 
     @Override
-    public CommandResult undo(Model model) throws CommandException {
-        super.canUndo();
+    protected CommandResult executeUndo(Model model) {
         model.setAddressBook(this.oldAddressBook);
         return new CommandResult(MESSAGE_SUCCESS);
     }

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

@@ -1,66 +1,18 @@
 package seedu.address.logic.commands;
 
-import seedu.address.commons.core.Messages;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 
 /**
  * Represents a command with hidden internal logic and the ability to be executed.
  */
-public abstract class Command {
-
-    protected boolean canExecute;
-
-    /**
-     * Default constructor for all classes that implement Command.
-     * @param canExecute Boolean for if the class is able to run the execute method.
-     */
-    public Command(boolean canExecute) {
-        this.canExecute = canExecute;
-    }
-
-    /**
-     * Constructor that initialises canExecute to be true when super is not called in child class.
-     */
-    public Command() {
-        this.canExecute = true;
-    }
-
+public interface Command {
     /**
      * Executes the command and returns the result message.
      *
      * @param model {@code Model} which the command should operate on.
      * @return feedback message of the operation result for display
      * @throws CommandException If an error occurs during command execution.
      */
-    public abstract CommandResult execute(Model model) throws CommandException;
-
-    /**
-     * Checks if the current object state is able to call execute(). If it is able to,
-     * allow execution and toggle canExecute, else throw CommandException.
-     * @throws CommandException to prevent execution.
-     */
-    public void canExecute() throws CommandException {
-        if (!this.canExecute) {
-            throw new CommandException(Messages.MESSAGE_UNABLE_TO_EXECUTE);
-        }
-        this.canExecute = false;
-    }
-
-    /**
-     * Checks if the current object state is able to call undo(). If it is able to,
-     * allow undo and toggle canExecute, else throw CommandException.
-     * @throws CommandException to prevent undo.
-     */
-    public void canUndo() throws CommandException {
-        if (this.canExecute) {
-            throw new CommandException(Messages.MESSAGE_UNABLE_TO_UNDO);
-        }
-        this.canExecute = true;
-    }
-
-    public CommandResult undo(Model model) throws CommandException {
-        return new CommandResult("Undo not supported for this command, or cannot undo any further");
-    }
-
+    CommandResult execute(Model model) throws CommandException;
 }

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

@@ -18,7 +18,7 @@
 /**
  * Deletes a person identified using it's displayed index from the address book.
  */
-public class DeleteCommand extends Command {
+public class DeleteCommand extends UndoableCommand {
 
     public static final String COMMAND_WORD = "delete";
 
@@ -41,34 +41,32 @@ public DeleteCommand(Index targetIndex) {
     }
 
     @Override
-    public CommandResult execute(Model model) throws CommandException {
+    protected CommandResult executeDo(Model model) throws CommandException {
         requireNonNull(model);
         List<Person> lastShownList = model.getFilteredPersonList();
 
         if (targetIndex.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
         }
-        super.canExecute();
         Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
         this.personToDelete = personToDelete;
         model.deletePerson(personToDelete);
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
 
     @Override
-    public boolean equals(Object other) {
-        return other == this // short circuit if same object
-                || (other instanceof DeleteCommand // instanceof handles nulls
-                && targetIndex.equals(((DeleteCommand) other).targetIndex)); // state check
-    }
-
-    @Override
-    public CommandResult undo(Model model) throws CommandException {
-        super.canUndo();
+    protected CommandResult executeUndo(Model model) {
         Predicate<? super Person> predicate = model.getFilteredPersonPredicate();
         model.addPerson(this.personToDelete);
         model.updateFilteredPersonList(predicate);
 
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
+
+    @Override
+    public boolean equals(Object other) {
+        return other == this // short circuit if same object
+                || (other instanceof DeleteCommand // instanceof handles nulls
+                && targetIndex.equals(((DeleteCommand) other).targetIndex)); // state check
+    }
 }

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

@@ -35,7 +35,7 @@
 /**
  * Edits the details of an existing person in the address book.
  */
-public class EditCommand extends Command {
+public class EditCommand extends UndoableCommand {
 
     public static final String COMMAND_WORD = "edit";
     public static final CommandSpecification COMMAND_SPECS = new CommandSpecification(
@@ -101,7 +101,7 @@ private static Person createEditedPerson(Person personToEdit, EditPersonDescript
     }
 
     @Override
-    public CommandResult execute(Model model) throws CommandException {
+    protected CommandResult executeDo(Model model) throws CommandException {
         requireNonNull(model);
         List<Person> lastShownList = model.getFilteredPersonList();
 
@@ -118,7 +118,6 @@ public CommandResult execute(Model model) throws CommandException {
         if (!personToEdit.isSamePerson(editedPerson) && model.hasPerson(editedPerson)) {
             throw new CommandException(MESSAGE_DUPLICATE_PERSON);
         }
-        super.canExecute();
 
         model.setPerson(personToEdit,
                 editedPerson);
@@ -128,8 +127,7 @@ public CommandResult execute(Model model) throws CommandException {
     }
 
     @Override
-    public CommandResult undo(Model model) throws CommandException {
-        super.canUndo();
+    protected CommandResult executeUndo(Model model) {
         EditPersonDescriptor oldDescriptor = new EditPersonDescriptor(this.originalPerson);
         Person originalPerson = createEditedPerson(this.editedPerson,
                 oldDescriptor);
@@ -154,7 +152,6 @@ public boolean equals(Object other) {
         // state check
         EditCommand e = (EditCommand) other;
         return index.equals(e.index) && editPersonDescriptor.equals(e.editPersonDescriptor);
-
     }
 
     /**

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

@@ -6,7 +6,7 @@
 /**
  * Terminates the program.
  */
-public class ExitCommand extends Command {
+public class ExitCommand implements Command {
 
     public static final String COMMAND_WORD = "exit";