Skip to content

Commit

Permalink
Merge pull request #214 from charlesliyifeng/refactor-undoable-as-int…
Browse files Browse the repository at this point in the history
…erface

Refactor undoable as interface and implement undo for import
  • Loading branch information
SeanFoongjt authored Nov 7, 2024
2 parents c0ea158 + 45c0af9 commit a060247
Show file tree
Hide file tree
Showing 15 changed files with 54 additions and 42 deletions.
12 changes: 6 additions & 6 deletions docs/DeveloperGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ The `Model` component,
* stores the address book data i.e., all `Person` objects (which are contained in a `UniquePersonList` object).
* stores the currently 'selected' `Person` objects (e.g., results of a search query) as a separate _filtered_ list which is exposed to outsiders as an unmodifiable `ObservableList<Person>` that can be 'observed' e.g. the UI can be bound to this list so that the UI automatically updates when the data in the list change.
* stores a `UserPref` object that represents the user’s preferences. This is exposed to the outside as a `ReadOnlyUserPref` objects.
* stores a history of concrete commands (commands that can be undone) executed successfully by the user, which allows the user to undo commands.
* stores a history of undoable commands executed successfully by the user, which allows the user to undo commands.
* only depends on the Logic component due to the undo feature.

<box type="info" seamless>
Expand Down Expand Up @@ -163,12 +163,12 @@ This section describes some noteworthy details on how certain features are imple

#### Implementation

The undo mechanism is facilitated by the abstract class `ConcreteCommand`.
It extends `Command` and has the `undo()` method. The `undo()` method is called when the user executes the `undo` command.
The undo mechanism is facilitated by the interface `Undoable`.
It has the `undo()` method. The `undo()` method is called when the user executes the `undo` command.
The `undo()` method reverses the effects of the command that was previously executed.
The `undo()` method is implemented in the concrete command classes, such as `AddCommand`, `DeleteCommand`, `EditCommand`, etc.
The `undo()` method is implemented in the undoable command classes, such as `AddCommand`, `DeleteCommand`, `EditCommand`, etc.

The `Model` component stores a history of executed concrete commands in a stack.
The `Model` component stores a history of executed undoable commands in a stack.
When a command is executed successfully, the command is pushed onto the stack.
When the user executes the `undo` command, the `Model` component pops the last command from the stack and calls the `undo()` method of the command.

Expand Down Expand Up @@ -236,7 +236,7 @@ _{more aspects and alternatives to be added}_

#### Implementation

The `clean` command extends `ConcreteCommand`. The `clean` command deletes the contacts whose `GradYear` field is earlier
The `clean` command extends `Command` and implements `Undoable`. The `clean` command deletes the contacts whose `GradYear` field is earlier
than the current year, deleting contacts who have graduated from the address book.
The `clean` command is undoable.

Expand Down
2 changes: 1 addition & 1 deletion docs/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ This deletes all students who graduate in 2023 or earlier.

Undoes the previous command that changes the data in the address book.

Commands that can be undone are `add`, `delete`, `edit`, `clear`, `clean`.
Commands that can be undone are `add`, `delete`, `edit`, `clear`, `clean`, `import`.

Format: `undo`

Expand Down
18 changes: 9 additions & 9 deletions docs/diagrams/LogicClassDiagram.puml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ package "Parser Classes" as ParserClasses{
}
Class A_Command
Class B_Command
Class C_Command
Class ImportCommand
Class ExportCommand
Class CommandResult
Class "{abstract}\nCommand" as Command
Class "{abstract}\nConcreteCommand" as ConcreteCommand
Class "<<interface>>\nUndoable" as Undoable
Class "{abstract}\nFileAccessCommand" as FileAccessCommand


Expand All @@ -35,14 +36,14 @@ LogicManager .right.|> Logic
LogicManager -right->"1" ParserClasses
ParserClasses ..> A_Command : <<create>>
ParserClasses ..> B_Command : <<create>>
ParserClasses ..> C_Command : <<create>>

ConcreteCommand -up-|> Command
FileAccessCommand -up-|> Command
A_Command -up-|> ConcreteCommand
A_Command .up.|> Undoable
A_Command -up-|> Command
B_Command -up-|> Command
C_Command -up-|> FileAccessCommand
A_Command .[hidden]right.> C_Command
ExportCommand -up-|> FileAccessCommand
ImportCommand -up-|> FileAccessCommand
ImportCommand .up.|> Undoable
LogicManager .left.> Command : <<call>>

LogicManager --> Model
Expand All @@ -51,10 +52,9 @@ FileAccessCommand .up.> Storage
Storage --[hidden] Model
Command .[hidden]up.> Storage
Command .right.> Model
Model ..> Undoable
note bottom of A_Command: Commands that can be undone: \nAddCommand, DeleteCommand, etc.
note right of B_Command: Commands that cannot be undone: \nListCommand, UndoCommand, etc.
note right of C_Command: Commands that access Storage: \nExportCommand, ImportCommand, etc.


Logic ..> CommandResult
LogicManager .down.> CommandResult
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/seedu/address/logic/LogicManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +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.ConcreteCommand;
import seedu.address.logic.commands.FileAccessCommand;
import seedu.address.logic.commands.ImportCommand;
import seedu.address.logic.commands.Undoable;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.logic.parser.AddressBookParser;
import seedu.address.logic.parser.exceptions.ParseException;
Expand Down Expand Up @@ -59,12 +58,10 @@ public CommandResult execute(String commandText) throws CommandException, ParseE
assert command.isExecuted() : "Command should be executed.";
assert commandResult != null : "CommandResult should not be null.";

// Push the command to the undo stack if it is a ConcreteCommand and successfully executed
// Push the command to the undo stack if it is undoable and successfully executed
// No need to check for success as the command will exit through an exception if it fails
if (command instanceof ConcreteCommand) {
model.pushToUndoStack((ConcreteCommand) command);
} else if (command instanceof ImportCommand) {
model.clearUndoStack();
if (command instanceof Undoable) {
model.pushToUndoStack((Undoable) command);
}

try {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/**
* Adds a person to the address book.
*/
public class AddCommand extends ConcreteCommand {
public class AddCommand extends Command implements Undoable {

public static final String COMMAND_WORD = "add";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/**
* Deletes all people whose graduation date has passed.
*/
public class CleanCommand extends ConcreteCommand {
public class CleanCommand extends Command implements Undoable {

public static final String COMMAND_WORD = "clean";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* Clears the address book.
*/
public class ClearCommand extends ConcreteCommand {
public class ClearCommand extends Command implements Undoable {

public static final String COMMAND_WORD = "clear";
public static final String MESSAGE_SUCCESS = "Address book has been cleared!";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/**
* Deletes a person identified using its displayed index from the address book.
*/
public class DeleteCommand extends ConcreteCommand {
public class DeleteCommand extends Command implements Undoable {

public static final String COMMAND_WORD = "delete";
public static final String MESSAGE_USAGE = COMMAND_WORD
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
/**
* Edits the details of an existing person in the address book.
*/
public class EditCommand extends ConcreteCommand {
public class EditCommand extends Command implements Undoable {

public static final String COMMAND_WORD = "edit";

Expand Down
19 changes: 17 additions & 2 deletions src/main/java/seedu/address/logic/commands/ImportCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@
import seedu.address.commons.exceptions.DataLoadingException;
import seedu.address.commons.util.ToStringBuilder;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.AddressBook;
import seedu.address.model.Model;
import seedu.address.model.ReadOnlyAddressBook;
import seedu.address.storage.Storage;

/**
* Imports the contacts from a file to the address book.
*/
public class ImportCommand extends FileAccessCommand {
public class ImportCommand extends FileAccessCommand implements Undoable {
public static final String COMMAND_WORD = "import";
public static final String MESSAGE_SUCCESS = "Address book from %s has been imported!";
public static final String MESSAGE_UNDO_SUCCESS = "Address book has been restored to version before import!";
public static final String MESSAGE_USAGE = COMMAND_WORD
+ ": Imports the contacts found in the provided file into the address book. \n"
+ "Parameters: " + PREFIX_FILE_PATH + "FILE_PATH\n"
Expand All @@ -27,6 +29,7 @@ public class ImportCommand extends FileAccessCommand {
"Could not read data from file %s due to inability to find or access the file.";

private final Path filePath;
private AddressBook initialAddressBook;

/**
* @param filePath of the json file to be imported
Expand All @@ -42,11 +45,12 @@ public CommandResult execute(Model model, Storage storage) throws CommandExcepti
requireNotExecuted();
requireNonNull(model);
requireNonNull(storage);
isExecuted = true;

Optional<ReadOnlyAddressBook> addressBookOptional;
ReadOnlyAddressBook importedData;

initialAddressBook = new AddressBook(model.getAddressBook());

try {
addressBookOptional = storage.readAddressBook(filePath);
importedData = addressBookOptional.orElseGet(model::getAddressBook);
Expand All @@ -55,9 +59,20 @@ public CommandResult execute(Model model, Storage storage) throws CommandExcepti
}

model.setAddressBook(importedData);
isExecuted = true;
return new CommandResult(String.format(MESSAGE_SUCCESS, this.filePath));
}

@Override
public CommandResult undo(Model model) {
requireExecuted();
requireNonNull(model);
requireNonNull(initialAddressBook);
model.setAddressBook(initialAddressBook);
isExecuted = false;
return new CommandResult(MESSAGE_UNDO_SUCCESS);

Check warning on line 73 in src/main/java/seedu/address/logic/commands/ImportCommand.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/logic/commands/ImportCommand.java#L68-L73

Added lines #L68 - L73 were not covered by tests
}

@Override
public boolean equals(Object other) {
if (other == this) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import seedu.address.model.Model;

/**
* Undoes the previous concrete command.
* Undoes the previous undoable command.
*/
public class UndoCommand extends Command {
public static final String COMMAND_WORD = "undo";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
/**
* Represents a command which can be undone by the undo command.
*/
public abstract class ConcreteCommand extends Command {
public interface Undoable {
/**
* Undoes the effects of 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
*/
public abstract CommandResult undo(Model model);
CommandResult undo(Model model);
}
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/model/Model.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import seedu.address.commons.core.GuiSettings;
import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.CommandResult;
import seedu.address.logic.commands.ConcreteCommand;
import seedu.address.logic.commands.Undoable;
import seedu.address.model.person.Person;

/**
Expand Down Expand Up @@ -116,12 +116,12 @@ public interface Model {
void updateFilteredPersonList(Predicate<Person> predicate);

/**
* Pushes a concrete command to the undo stack.
* Pushes an undoable command to the undo stack.
*/
void pushToUndoStack(ConcreteCommand command);
void pushToUndoStack(Undoable command);

/**
* Undoes the most recent concrete command.
* Undoes the most recent undoable command.
*/
CommandResult undoAddressBook();

Expand Down
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/model/ModelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import seedu.address.commons.core.LogsCenter;
import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.CommandResult;
import seedu.address.logic.commands.ConcreteCommand;
import seedu.address.logic.commands.Undoable;
import seedu.address.model.person.Person;

/**
Expand All @@ -26,7 +26,7 @@ public class ModelManager implements Model {
private final AddressBook addressBook;
private final UserPrefs userPrefs;
private final FilteredList<Person> filteredPersons;
private final Stack<ConcreteCommand> undoStack;
private final Stack<Undoable> undoStack;

/**
* Initializes a ModelManager with the given addressBook and userPrefs.
Expand Down Expand Up @@ -182,7 +182,7 @@ public boolean equals(Object other) {
//=========== Undo ================================================================================

@Override
public void pushToUndoStack(ConcreteCommand command) {
public void pushToUndoStack(Undoable command) {
assert command != null;
undoStack.push(command);
}
Expand All @@ -192,7 +192,7 @@ public CommandResult undoAddressBook() {
if (undoStack.isEmpty()) {
return null;
}
ConcreteCommand command = undoStack.pop();
Undoable command = undoStack.pop();
return command.undo(this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public void updateFilteredPersonList(Predicate<Person> predicate) {
}

@Override
public void pushToUndoStack(ConcreteCommand command) {
public void pushToUndoStack(Undoable command) {
throw new AssertionError("This method should not be called.");
}

Expand Down

0 comments on commit a060247

Please sign in to comment.