From 1c6265b229604fb1a01e5a13412a52af6292f0dd Mon Sep 17 00:00:00 2001 From: rachx Date: Mon, 7 Nov 2016 00:14:57 +0800 Subject: [PATCH] more minor refactoring (#211) * extract method in model manager * update comments rename toDoList * update messages (remove - in angle bracket, new message for alias), remove some code from undo in logic manager test --- .../agendum/logic/commands/AddCommand.java | 2 +- .../agendum/logic/commands/AliasCommand.java | 6 +- .../agendum/logic/commands/DeleteCommand.java | 2 +- .../agendum/logic/commands/FindCommand.java | 2 +- .../agendum/logic/commands/MarkCommand.java | 2 +- .../agendum/logic/commands/RenameCommand.java | 2 +- .../logic/commands/ScheduleCommand.java | 2 +- .../logic/commands/UnaliasCommand.java | 7 ++- src/main/java/seedu/agendum/model/Model.java | 2 +- .../seedu/agendum/model/ModelManager.java | 62 ++++++++++--------- .../seedu/agendum/logic/LogicManagerTest.java | 26 +++----- 11 files changed, 59 insertions(+), 56 deletions(-) diff --git a/src/main/java/seedu/agendum/logic/commands/AddCommand.java b/src/main/java/seedu/agendum/logic/commands/AddCommand.java index 664a7be6f8e2..3c60b283b5bb 100644 --- a/src/main/java/seedu/agendum/logic/commands/AddCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/AddCommand.java @@ -16,7 +16,7 @@ public class AddCommand extends Command { public static final String COMMAND_FORMAT = "add \n" + "add by \n" - + "add from to "; + + "add from to "; public static final String COMMAND_DESCRIPTION = "adds a task to Agendum"; public static final String MESSAGE_SUCCESS = "Task added: %1$s"; public static final String MESSAGE_DUPLICATE_TASK = "Hey, the task already exists"; diff --git a/src/main/java/seedu/agendum/logic/commands/AliasCommand.java b/src/main/java/seedu/agendum/logic/commands/AliasCommand.java index bb6496837ceb..933f841bc82a 100644 --- a/src/main/java/seedu/agendum/logic/commands/AliasCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/AliasCommand.java @@ -9,11 +9,11 @@ public class AliasCommand extends Command { public static final String COMMAND_WORD = "alias"; - public static final String COMMAND_FORMAT = "alias "; + public static final String COMMAND_FORMAT = "alias "; public static final String COMMAND_DESCRIPTION = "create your shorthand command"; public static final String MESSAGE_SUCCESS = "New alias <%1$s> created for <%2$s>"; public static final String MESSAGE_FAILURE_ALIAS_IN_USE = "<%1$s> is already an alias for <%2$s>"; - public static final String MESSAGE_FAILURE_UNAVAILABLE_ALIAS = "<%1$s> is a reserved command word"; + public static final String MESSAGE_FAILURE_RESERVED_COMMAND_WORD = "<%1$s> is a reserved command word"; public static final String MESSAGE_FAILURE_NON_ORIGINAL_COMMAND = "We don't recognise <%1$s> as an Agendum Command"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " @@ -39,7 +39,7 @@ public void setData(Model model, CommandLibrary commandLibrary) { public CommandResult execute() { if (commandLibrary.isReservedCommandKeyword(aliasKey)) { return new CommandResult(String.format( - MESSAGE_FAILURE_UNAVAILABLE_ALIAS, aliasKey)); + MESSAGE_FAILURE_RESERVED_COMMAND_WORD, aliasKey)); } if (commandLibrary.isExistingAliasKey(aliasKey)) { diff --git a/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java b/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java index 5ffde605ee7a..5e0ea9bbdaa7 100644 --- a/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java @@ -16,7 +16,7 @@ public class DeleteCommand extends Command { public static final String COMMAND_WORD = "delete"; - public static final String COMMAND_FORMAT = "delete "; + public static final String COMMAND_FORMAT = "delete "; public static final String COMMAND_DESCRIPTION = "delete task(s) from Agendum"; public static final String MESSAGE_DELETE_TASK_SUCCESS = "Deleted Task(s): %1$s"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " diff --git a/src/main/java/seedu/agendum/logic/commands/FindCommand.java b/src/main/java/seedu/agendum/logic/commands/FindCommand.java index 80dd6b474955..9656291d5744 100644 --- a/src/main/java/seedu/agendum/logic/commands/FindCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/FindCommand.java @@ -10,7 +10,7 @@ public class FindCommand extends Command { // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "find"; - public static final String COMMAND_FORMAT= "find "; + public static final String COMMAND_FORMAT= "find "; public static final String COMMAND_DESCRIPTION = "search for task(s) matching any of the keywords"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" diff --git a/src/main/java/seedu/agendum/logic/commands/MarkCommand.java b/src/main/java/seedu/agendum/logic/commands/MarkCommand.java index b34e55e32afb..c236285a87ba 100644 --- a/src/main/java/seedu/agendum/logic/commands/MarkCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/MarkCommand.java @@ -17,7 +17,7 @@ public class MarkCommand extends Command { public static final String COMMAND_WORD = "mark"; - public static final String COMMAND_FORMAT = "mark "; + public static final String COMMAND_FORMAT = "mark "; public static final String COMMAND_DESCRIPTION = "mark task(s) as completed"; public static final String MESSAGE_MARK_TASK_SUCCESS = "Marked Task(s)!"; diff --git a/src/main/java/seedu/agendum/logic/commands/RenameCommand.java b/src/main/java/seedu/agendum/logic/commands/RenameCommand.java index ae4ed62be2b6..37364c356a91 100644 --- a/src/main/java/seedu/agendum/logic/commands/RenameCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/RenameCommand.java @@ -13,7 +13,7 @@ public class RenameCommand extends Command { public static final String COMMAND_WORD = "rename"; - public static final String COMMAND_FORMAT = "rename "; + public static final String COMMAND_FORMAT = "rename "; public static final String COMMAND_DESCRIPTION = "update the name of a task"; public static final String MESSAGE_SUCCESS = "Task renamed: %1$s"; public static final String MESSAGE_DUPLICATE_TASK = "Hey, the task already exists"; diff --git a/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java b/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java index a8d1906cecff..936f559e767d 100644 --- a/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java @@ -17,7 +17,7 @@ public class ScheduleCommand extends Command { public static final String COMMAND_WORD = "schedule"; public static final String COMMAND_FORMAT = "schedule \n" + "schedule by \n" - + "schedule from to "; + + "schedule from to "; public static final String COMMAND_DESCRIPTION = "update the time of a task"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " diff --git a/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java b/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java index 936e999d7596..d8c1b108d762 100644 --- a/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java @@ -9,11 +9,12 @@ public class UnaliasCommand extends Command { public static final String COMMAND_WORD = "unalias"; - public static final String COMMAND_FORMAT = "unalias "; + public static final String COMMAND_FORMAT = "unalias "; public static final String COMMAND_DESCRIPTION = "remove a shorthand command"; public static final String MESSAGE_SUCCESS = "Removed alias <%1$s>"; public static final String MESSAGE_FAILURE_NO_ALIAS_KEY = "The alias <%1$s> does not exist"; + public static final String MESSAGE_FAILURE_RESERVED_COMMAND_WORD = "<%1$s> is a reserved command word"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" @@ -34,6 +35,10 @@ public void setData(Model model, CommandLibrary commandLibrary) { @Override public CommandResult execute() { + if (commandLibrary.isReservedCommandKeyword(aliasKey)) { + return new CommandResult(String.format(MESSAGE_FAILURE_RESERVED_COMMAND_WORD, aliasKey)); + } + if (!commandLibrary.isExistingAliasKey(aliasKey)) { return new CommandResult(String.format(MESSAGE_FAILURE_NO_ALIAS_KEY, aliasKey)); } diff --git a/src/main/java/seedu/agendum/model/Model.java b/src/main/java/seedu/agendum/model/Model.java index 355e76c21821..aced4fed5ad9 100644 --- a/src/main/java/seedu/agendum/model/Model.java +++ b/src/main/java/seedu/agendum/model/Model.java @@ -44,7 +44,7 @@ void unmarkTasks(List targets) void restorePreviousToDoList() throws NoPreviousListFoundException; /** - * Resets the to-do list data to match the top list in the stack of previous lists + * Resets the main to-do list data to match the top list in the stack of previous lists */ void resetDataToLastSavedList(); diff --git a/src/main/java/seedu/agendum/model/ModelManager.java b/src/main/java/seedu/agendum/model/ModelManager.java index c7d579eb81ed..7d7cc0fba213 100644 --- a/src/main/java/seedu/agendum/model/ModelManager.java +++ b/src/main/java/seedu/agendum/model/ModelManager.java @@ -33,7 +33,7 @@ public class ModelManager extends ComponentManager implements Model { private static final Logger logger = LogsCenter.getLogger(ModelManager.class); - private final ToDoList toDoList; + private final ToDoList mainToDoList; private final Stack previousLists; private final FilteredList filteredTasks; private final SortedList sortedTasks; @@ -59,8 +59,8 @@ public ModelManager(ToDoList src, UserPrefs userPrefs) { logger.fine("Initializing with to do list: " + src + " and user prefs " + userPrefs); - toDoList = new ToDoList(src); - filteredTasks = new FilteredList<>(toDoList.getTasks()); + mainToDoList = new ToDoList(src); + filteredTasks = new FilteredList<>(mainToDoList.getTasks()); sortedTasks = filteredTasks.sorted(); previousLists = new Stack(); backupCurrentToDoList(); @@ -73,8 +73,8 @@ public ModelManager() { } public ModelManager(ReadOnlyToDoList initialData) { - toDoList = new ToDoList(initialData); - filteredTasks = new FilteredList(toDoList.getTasks()); + mainToDoList = new ToDoList(initialData); + filteredTasks = new FilteredList(mainToDoList.getTasks()); sortedTasks = filteredTasks.sorted(); previousLists = new Stack(); backupCurrentToDoList(); @@ -85,7 +85,7 @@ public ModelManager(ReadOnlyToDoList initialData) { //@@author A0133367E @Override public void resetData(ReadOnlyToDoList newData) { - toDoList.resetData(newData); + mainToDoList.resetData(newData); logger.fine("[MODEL] --- successfully reset data of the to-do list"); backupCurrentToDoList(); indicateToDoListChanged(); @@ -94,14 +94,14 @@ public void resetData(ReadOnlyToDoList newData) { @Override public ReadOnlyToDoList getToDoList() { - return toDoList; + return mainToDoList; } /** Raises an event to indicate the model has changed */ private void indicateToDoListChanged() { // force a reset/refresh for list view in UI - toDoList.resetData(toDoList); - raise(new ToDoListChangedEvent(toDoList)); + mainToDoList.resetData(mainToDoList); + raise(new ToDoListChangedEvent(mainToDoList)); } //@@author A0148095X @@ -119,10 +119,8 @@ private void indicateLoadDataRequest(String location) { @Override public synchronized void deleteTasks(List targets) throws TaskNotFoundException { for (ReadOnlyTask target: targets) { - toDoList.removeTask(target); - - // Delete tasks in sync manager - syncManager.deleteEvent((Task) target); + mainToDoList.removeTask(target); + removeTaskFromSyncManager(target); } logger.fine("[MODEL] --- successfully deleted all specified targets from the to-do list"); @@ -132,37 +130,35 @@ public synchronized void deleteTasks(List targets) throws TaskNotF @Override public synchronized void addTask(Task task) throws UniqueTaskList.DuplicateTaskException { - toDoList.addTask(task); + mainToDoList.addTask(task); logger.fine("[MODEL] --- successfully added the new task to the to-do list"); backupCurrentToDoList(); updateFilteredListToShowAll(); indicateToDoListChanged(); - - syncManager.addNewEvent(task); + addTaskToSyncManager(task); } @Override public synchronized void updateTask(ReadOnlyTask target, Task updatedTask) throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { - toDoList.updateTask(target, updatedTask); + mainToDoList.updateTask(target, updatedTask); logger.fine("[MODEL] --- successfully updated the target task in the to-do list"); backupCurrentToDoList(); updateFilteredListToShowAll(); indicateToDoListChanged(); - // Delete old task and add new task - syncManager.deleteEvent((Task) target); - syncManager.addNewEvent(updatedTask); + addTaskToSyncManager(updatedTask); + removeTaskFromSyncManager(target); } @Override public synchronized void markTasks(List targets) throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { for (ReadOnlyTask target: targets) { - toDoList.markTask(target); - } + mainToDoList.markTask(target); + } logger.fine("[MODEL] --- successfully marked all specified targets from the to-do list"); backupCurrentToDoList(); @@ -173,7 +169,7 @@ public synchronized void markTasks(List targets) public synchronized void unmarkTasks(List targets) throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { for (ReadOnlyTask target: targets) { - toDoList.unmarkTask(target); + mainToDoList.unmarkTask(target); } logger.fine("[MODEL] --- successfully unmarked all specified targets from the to-do list"); @@ -193,13 +189,13 @@ public synchronized void restorePreviousToDoList() throws NoPreviousListFoundExc } /** - * Resets the to-do list data to match the top list in the stack of previous lists + * Resets the {@code mainToDoList} data to match the top list in the {@code previousLists} stack */ @Override public void resetDataToLastSavedList() { assert !previousLists.empty(); ToDoList lastSavedListFromHistory = previousLists.peek(); - toDoList.resetData(lastSavedListFromHistory); + mainToDoList.resetData(lastSavedListFromHistory); } private void backupCurrentToDoList() { @@ -212,7 +208,7 @@ private void clearAllPreviousToDoLists() { } /** - * Pops the top list from the stack of previous lists if there are more than 1 list in the stack + * Pops the top list from the {@code previousLists} stack if there are more than 1 list present * @throws NoPreviousListFoundException if there is only 1 list in the stack */ private void removeLastSavedToDoList() throws NoPreviousListFoundException { @@ -244,6 +240,14 @@ public synchronized void loadFromLocation(String location) { indicateChangeSaveLocation(location); indicateLoadDataRequest(location); } + + private void addTaskToSyncManager(Task task) { + syncManager.addNewEvent(task); + } + + private void removeTaskFromSyncManager(ReadOnlyTask task) { + syncManager.deleteEvent((Task) task); + } //@@author A0003878Y //=========== Sync Methods =============================================================================== @@ -254,7 +258,7 @@ public void activateModelSyncing() { syncManager.startSyncing(); // Add all current events into sync provider - toDoList.getTasks().forEach(syncManager::addNewEvent); + mainToDoList.getTasks().forEach(syncManager::addNewEvent); } } @@ -265,6 +269,8 @@ public void deactivateModelSyncing() { } } + + //@@author //=========== Filtered Task List Accessors =============================================================== @@ -344,7 +350,7 @@ public String toString() { @Override @Subscribe public void handleLoadDataCompleteEvent(LoadDataCompleteEvent event) { - this.toDoList.resetData(event.data); + this.mainToDoList.resetData(event.data); indicateToDoListChanged(); clearAllPreviousToDoLists(); backupCurrentToDoList(); diff --git a/src/test/java/seedu/agendum/logic/LogicManagerTest.java b/src/test/java/seedu/agendum/logic/LogicManagerTest.java index eef0825edb50..0ed3b28e7086 100644 --- a/src/test/java/seedu/agendum/logic/LogicManagerTest.java +++ b/src/test/java/seedu/agendum/logic/LogicManagerTest.java @@ -832,7 +832,7 @@ public void execute_aliasNonOriginalCommand_errorMessageShown() throws Exception @Test public void execute_aliasKeyIsReservedCommandWord_errorMessageShown() throws Exception { - String expectedMessage = String.format(AliasCommand.MESSAGE_FAILURE_UNAVAILABLE_ALIAS, + String expectedMessage = String.format(AliasCommand.MESSAGE_FAILURE_RESERVED_COMMAND_WORD, RenameCommand.COMMAND_WORD); String userCommand = "alias " + AddCommand.COMMAND_WORD + " " + RenameCommand.COMMAND_WORD; assertCommandBehavior(userCommand, expectedMessage, new ToDoList(), Collections.emptyList()); @@ -875,6 +875,14 @@ public void execute_unaliasInvalidArgsFormat_errorMessageShown() throws Exceptio assertCommandBehavior("unalias", expectedMessage, new ToDoList(), Collections.emptyList()); } + @Test + public void execute_unaliasReservedCommandWord_errorMessageShown() throws Exception { + String expectedMessage = String.format(UnaliasCommand.MESSAGE_FAILURE_RESERVED_COMMAND_WORD, + AddCommand.COMMAND_WORD); + String command = "unalias " + AddCommand.COMMAND_WORD; + assertCommandBehavior(command, expectedMessage, new ToDoList(), Collections.emptyList()); + } + @Test public void execute_unaliasNoSuchKey_errorMessageShown() throws Exception { String expectedMessage = String.format(UnaliasCommand.MESSAGE_FAILURE_NO_ALIAS_KEY, "smth"); @@ -980,10 +988,6 @@ public void execute_undo_reversePreviousChangeToTaskList() throws Exception { model.deleteTasks(readOnlyTaskList); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - // Undo clear command - model.resetData(new ToDoList()); - assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - // Undo rename command Task p2 = new Task(p1); p2.setName(new Name("new name")); @@ -993,17 +997,6 @@ public void execute_undo_reversePreviousChangeToTaskList() throws Exception { // Undo mark command model.markTasks(readOnlyTaskList); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - - // Undo unmark command - model.markTasks(readOnlyTaskList); - Task p3 = new Task(p1); //p1 clone - p3.markAsCompleted(); - listWithOneTask = helper.generateTaskList(p3); - expectedTDL = helper.generateToDoList(listWithOneTask); - readOnlyTaskList = helper.generateReadOnlyTaskList(p3); - model.unmarkTasks(readOnlyTaskList); - assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - } @@ -1190,7 +1183,6 @@ private List generateTaskList(Task... tasks) { return Arrays.asList(tasks); } - //@@author A0133367E private List generateReadOnlyTaskList(ReadOnlyTask... tasks) { return Arrays.asList(tasks); }