Skip to content

Commit

Permalink
more minor refactoring (#211)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
rachx authored and burnflare committed Nov 6, 2016
1 parent 519a0b8 commit 1c6265b
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/main/java/seedu/agendum/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class AddCommand extends Command {

public static final String COMMAND_FORMAT = "add <name>\n"
+ "add <name> by <deadline> \n"
+ "add <name> from <start-time> to <end-time>";
+ "add <name> from <start time> to <end time>";
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";
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/seedu/agendum/logic/commands/AliasCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
public class AliasCommand extends Command {

public static final String COMMAND_WORD = "alias";
public static final String COMMAND_FORMAT = "alias <original-command> <your-command>";
public static final String COMMAND_FORMAT = "alias <original command> <your command>";
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 + " - "
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
public class DeleteCommand extends Command {

public static final String COMMAND_WORD = "delete";
public static final String COMMAND_FORMAT = "delete <id> <more-ids>";
public static final String COMMAND_FORMAT = "delete <id> <more ids>";
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 + " - "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <keyword> <more-keywords>";
public static final String COMMAND_FORMAT= "find <keyword> <more keywords>";
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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
public class MarkCommand extends Command {

public static final String COMMAND_WORD = "mark";
public static final String COMMAND_FORMAT = "mark <id> <more-ids>";
public static final String COMMAND_FORMAT = "mark <id> <more ids>";
public static final String COMMAND_DESCRIPTION = "mark task(s) as completed";

public static final String MESSAGE_MARK_TASK_SUCCESS = "Marked Task(s)!";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
public class RenameCommand extends Command {

public static final String COMMAND_WORD = "rename";
public static final String COMMAND_FORMAT = "rename <id> <new-name>";
public static final String COMMAND_FORMAT = "rename <id> <new name>";
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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class ScheduleCommand extends Command {
public static final String COMMAND_WORD = "schedule";
public static final String COMMAND_FORMAT = "schedule <id>\n"
+ "schedule <id> by <deadline>\n"
+ "schedule <id> from <start-time> to <end-time>";
+ "schedule <id> from <start time> to <end time>";
public static final String COMMAND_DESCRIPTION = "update the time of a task";

public static final String MESSAGE_USAGE = COMMAND_WORD + " - "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
public class UnaliasCommand extends Command {

public static final String COMMAND_WORD = "unalias";
public static final String COMMAND_FORMAT = "unalias <your-command>";
public static final String COMMAND_FORMAT = "unalias <your command>";
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"
Expand All @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/agendum/model/Model.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void unmarkTasks(List<ReadOnlyTask> 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();

Expand Down
62 changes: 34 additions & 28 deletions src/main/java/seedu/agendum/model/ModelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<ToDoList> previousLists;
private final FilteredList<Task> filteredTasks;
private final SortedList<Task> sortedTasks;
Expand All @@ -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<ToDoList>();
backupCurrentToDoList();
Expand All @@ -73,8 +73,8 @@ public ModelManager() {
}

public ModelManager(ReadOnlyToDoList initialData) {
toDoList = new ToDoList(initialData);
filteredTasks = new FilteredList<Task>(toDoList.getTasks());
mainToDoList = new ToDoList(initialData);
filteredTasks = new FilteredList<Task>(mainToDoList.getTasks());
sortedTasks = filteredTasks.sorted();
previousLists = new Stack<ToDoList>();
backupCurrentToDoList();
Expand All @@ -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();
Expand All @@ -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
Expand All @@ -119,10 +119,8 @@ private void indicateLoadDataRequest(String location) {
@Override
public synchronized void deleteTasks(List<ReadOnlyTask> 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");
Expand All @@ -132,37 +130,35 @@ public synchronized void deleteTasks(List<ReadOnlyTask> 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<ReadOnlyTask> 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();
Expand All @@ -173,7 +169,7 @@ public synchronized void markTasks(List<ReadOnlyTask> targets)
public synchronized void unmarkTasks(List<ReadOnlyTask> 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");
Expand All @@ -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() {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 ===============================================================================
Expand All @@ -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);
}
}

Expand All @@ -265,6 +269,8 @@ public void deactivateModelSyncing() {
}
}



//@@author
//=========== Filtered Task List Accessors ===============================================================

Expand Down Expand Up @@ -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();
Expand Down
26 changes: 9 additions & 17 deletions src/test/java/seedu/agendum/logic/LogicManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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"));
Expand All @@ -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);

}


Expand Down Expand Up @@ -1190,7 +1183,6 @@ private List<Task> generateTaskList(Task... tasks) {
return Arrays.asList(tasks);
}

//@@author A0133367E
private List<ReadOnlyTask> generateReadOnlyTaskList(ReadOnlyTask... tasks) {
return Arrays.asList(tasks);
}
Expand Down

0 comments on commit 1c6265b

Please sign in to comment.