From ceff16997a0e9271637917d37becca9af38983c2 Mon Sep 17 00:00:00 2001 From: rachx Date: Sun, 6 Nov 2016 14:23:21 +0800 Subject: [PATCH] Code refactoring for A0133367E (#198) * Remove redundant import and add header comments for methods in the Task class * Update read only task * Rename method in ModelManager * remove "this is" in model manager comments * update undo and previous lists related data/functions in model * Add _ to denote private variables in Task * Update alias table changed event * Remove redundant report and update comments in alias table storage files * Indentation formatting for todolist and readonlytask * indentation formatting for parser class * update parser comments * update commands and code claim * add white space * add spaces * update comments for command library and results * adjust indentation * update tests * fix failing tests * add author tag for gui test --- .../events/logic/AliasTableChangedEvent.java | 14 +- .../agendum/logic/commands/AddCommand.java | 8 +- .../agendum/logic/commands/AliasCommand.java | 13 +- .../logic/commands/CommandLibrary.java | 60 +++++-- .../agendum/logic/commands/CommandResult.java | 4 + .../agendum/logic/commands/DeleteCommand.java | 15 +- .../agendum/logic/commands/MarkCommand.java | 20 +-- .../agendum/logic/commands/RenameCommand.java | 21 +-- .../logic/commands/ScheduleCommand.java | 8 +- .../logic/commands/UnaliasCommand.java | 10 +- .../agendum/logic/commands/UndoCommand.java | 20 ++- .../agendum/logic/commands/UnmarkCommand.java | 16 +- .../seedu/agendum/logic/parser/Parser.java | 63 +++---- src/main/java/seedu/agendum/model/Model.java | 10 +- .../seedu/agendum/model/ModelManager.java | 75 ++++---- .../java/seedu/agendum/model/ToDoList.java | 14 +- .../agendum/model/task/ReadOnlyTask.java | 39 +++-- .../java/seedu/agendum/model/task/Task.java | 160 +++++++++++++----- .../agendum/storage/AliasTableStorage.java | 4 +- .../storage/JsonAliasTableStorage.java | 3 +- src/test/java/guitests/AddCommandTest.java | 9 +- src/test/java/guitests/ToDoListGuiTest.java | 6 +- .../seedu/agendum/logic/LogicManagerTest.java | 60 ++++--- .../java/seedu/agendum/model/TaskTest.java | 7 +- .../agendum/model/UniqueTaskListTest.java | 3 + 25 files changed, 391 insertions(+), 271 deletions(-) diff --git a/src/main/java/seedu/agendum/commons/events/logic/AliasTableChangedEvent.java b/src/main/java/seedu/agendum/commons/events/logic/AliasTableChangedEvent.java index cf01ab704437..791fd454327e 100644 --- a/src/main/java/seedu/agendum/commons/events/logic/AliasTableChangedEvent.java +++ b/src/main/java/seedu/agendum/commons/events/logic/AliasTableChangedEvent.java @@ -1,25 +1,25 @@ //@@author A0133367E package seedu.agendum.commons.events.logic; -import seedu.agendum.commons.events.BaseEvent; +import java.util.Hashtable; -import java.util.*; +import seedu.agendum.commons.events.BaseEvent; /** - * Indicate the alias table in Logic's command library has changed + * Indicate the alias table in {@link seedu.agendum.logic.commands.CommandLibrary} has changed */ public class AliasTableChangedEvent extends BaseEvent { - public final String aliasedKeyChanged; public final Hashtable aliasTable; + private String message_; - public AliasTableChangedEvent(String aliasedKeyChanged, Hashtable aliasTable) { - this.aliasedKeyChanged = aliasedKeyChanged; + public AliasTableChangedEvent(String message, Hashtable aliasTable) { this.aliasTable = aliasTable; + this.message_ = message; } @Override public String toString() { - return aliasedKeyChanged; + return message_; } } diff --git a/src/main/java/seedu/agendum/logic/commands/AddCommand.java b/src/main/java/seedu/agendum/logic/commands/AddCommand.java index 91c4fb9272ea..664a7be6f8e2 100644 --- a/src/main/java/seedu/agendum/logic/commands/AddCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/AddCommand.java @@ -18,19 +18,17 @@ public class AddCommand extends Command { + "add by \n" + "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"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" + "Example: " + COMMAND_WORD + " Watch Star Wars\n" + "from 7pm to 9pm"; - public static final String MESSAGE_SUCCESS = "Task added: %1$s"; - public static final String MESSAGE_DUPLICATE_TASK = "Hey, the task already exists"; - private Task toAdd = null; - //@@author A0003878Y + //@@author A0003878Y /** * Convenience constructor using name * diff --git a/src/main/java/seedu/agendum/logic/commands/AliasCommand.java b/src/main/java/seedu/agendum/logic/commands/AliasCommand.java index 4a7e4a55f89d..bb6496837ceb 100644 --- a/src/main/java/seedu/agendum/logic/commands/AliasCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/AliasCommand.java @@ -1,14 +1,13 @@ -//@@author A0133367E package seedu.agendum.logic.commands; import seedu.agendum.model.Model; +//@@author A0133367E /** - * Create an alias for a reserved command keyword + * Creates an alias for a reserved command keyword */ public class AliasCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "alias"; public static final String COMMAND_FORMAT = "alias "; public static final String COMMAND_DESCRIPTION = "create your shorthand command"; @@ -38,9 +37,9 @@ public void setData(Model model, CommandLibrary commandLibrary) { @Override public CommandResult execute() { - if (!commandLibrary.isReservedCommandKeyword(aliasValue)) { + if (commandLibrary.isReservedCommandKeyword(aliasKey)) { return new CommandResult(String.format( - MESSAGE_FAILURE_NON_ORIGINAL_COMMAND, aliasValue)); + MESSAGE_FAILURE_UNAVAILABLE_ALIAS, aliasKey)); } if (commandLibrary.isExistingAliasKey(aliasKey)) { @@ -49,9 +48,9 @@ public CommandResult execute() { MESSAGE_FAILURE_ALIAS_IN_USE, aliasKey, associatedValue)); } - if (commandLibrary.isReservedCommandKeyword(aliasKey)) { + if (!commandLibrary.isReservedCommandKeyword(aliasValue)) { return new CommandResult(String.format( - MESSAGE_FAILURE_UNAVAILABLE_ALIAS, aliasKey)); + MESSAGE_FAILURE_NON_ORIGINAL_COMMAND, aliasValue)); } commandLibrary.addNewAlias(aliasKey, aliasValue); diff --git a/src/main/java/seedu/agendum/logic/commands/CommandLibrary.java b/src/main/java/seedu/agendum/logic/commands/CommandLibrary.java index 159c6a16be4e..e2a008590c2d 100644 --- a/src/main/java/seedu/agendum/logic/commands/CommandLibrary.java +++ b/src/main/java/seedu/agendum/logic/commands/CommandLibrary.java @@ -1,5 +1,3 @@ -//@@author A0133367E - package seedu.agendum.logic.commands; import java.util.ArrayList; @@ -19,13 +17,15 @@ public class CommandLibrary { private static final Logger logger = LogsCenter.getLogger(CommandLibrary.class); + private static CommandLibrary commandLibrary = new CommandLibrary(); + private List allCommandWords = new ArrayList(); // The keys of the hash table are user-defined aliases // The values of the has table are Agendum's reserved command keywords private Hashtable aliasTable = new Hashtable(); - private static CommandLibrary commandLibrary = new CommandLibrary(); + //@@author A0003878Y private CommandLibrary() { @@ -49,9 +49,13 @@ public static CommandLibrary getInstance() { return commandLibrary; } + public Hashtable getAliasTable() { + return aliasTable; + } + //@@author A0133367E /** - * Replace the current commandLibrary's aliasTable with the aliasTable provided + * Replace the current commandLibrary's aliasTable with the new aliasTable provided */ public void loadAliasTable(Hashtable aliasTable) { this.aliasTable = aliasTable; @@ -68,8 +72,10 @@ public boolean isExistingAliasKey(String key) { } /** - * Precondition: key is an existing alias. * Returns the reserved command keyword that is aliased by key + * + * @param key An existing user-defined alias for a reserved command keyword + * @return The associated reserved command keyword */ public String getAliasedValue(String key) { assert isExistingAliasKey(key); @@ -88,9 +94,12 @@ public boolean isReservedCommandKeyword(String value) { } /** - * Precondition: key is a new unique alias and not a command keyword; + * Pre-condition: key is a new unique alias and not a command keyword; * value is a reserved command keyword. - * Saves the new alias relationship (key can be used in place of value) + * Saves the new alias relationship between key and value. + * + * @param key A valid and unique user-defined alias for a reserved command word + * @param value The target reserved command word */ public void addNewAlias(String key, String value) { assert !isExistingAliasKey(key); @@ -99,29 +108,44 @@ public void addNewAlias(String key, String value) { aliasTable.put(key, value); - indicateAliasTableChanged(key + " aliased"); + indicateAliasAdded(key, value); } /** - * Precondition: key is aliased to a command keyword. - * Destroy the alias relationship (key can no longer be used in place of command keyword) + * Destroy the alias relationship (key can no longer be used in place of command word) + * + * @param key An existing user-defined alias for a reserved command word */ public void removeExistingAlias(String key) { assert isExistingAliasKey(key); - aliasTable.remove(key); + String value = aliasTable.remove(key); - indicateAliasTableChanged(key + " unaliased"); + indicateAliasRemoved(key, value); } - /** Raises an event to indicate that the aliasTable in the command library has changed */ - private void indicateAliasTableChanged(String keyChanged) { + /** + * Raises an event to indicate that an alias has been added to aliasTable in the command library + * + * @param key The new user-defined alias key + * @param value The target reserved command word + */ + private void indicateAliasAdded(String key, String value) { + String message = "Added alias " + key + " for " + value; EventsCenter eventCenter = EventsCenter.getInstance(); - eventCenter.post(new AliasTableChangedEvent(keyChanged, aliasTable)); + eventCenter.post(new AliasTableChangedEvent(message, aliasTable)); } - - public Hashtable getAliasTable() { - return aliasTable; + + /** + * Raises an event to indicate that an alias has been removed from aliasTable in the command library + * + * @param key The alias key to be removed + * @param value The associated reserved command word + */ + private void indicateAliasRemoved(String key, String value) { + String message = "Removed alias " + key + " for " + value; + EventsCenter eventCenter = EventsCenter.getInstance(); + eventCenter.post(new AliasTableChangedEvent(message, aliasTable)); } } diff --git a/src/main/java/seedu/agendum/logic/commands/CommandResult.java b/src/main/java/seedu/agendum/logic/commands/CommandResult.java index 63807ad81d23..3941d4ea1bf0 100644 --- a/src/main/java/seedu/agendum/logic/commands/CommandResult.java +++ b/src/main/java/seedu/agendum/logic/commands/CommandResult.java @@ -22,6 +22,10 @@ public CommandResult(String feedbackToUser) { * Pre-condition: tasks and originalIndices must be of the same size. * Returns a string containing each task in tasks * with the corresponding number in originalIndices prepended + * + * @param tasks List of tasks where each task is be prepended by an index + * @param originalIndices List of corresponding index for each task + * @return String containing all tasks labeled with their corresponding index */ public static String tasksToString(List tasks, List originalIndices) { final StringBuilder builder = new StringBuilder(); diff --git a/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java b/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java index b02c51ef8301..5ffde605ee7a 100644 --- a/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/DeleteCommand.java @@ -9,32 +9,30 @@ import seedu.agendum.model.task.ReadOnlyTask; import seedu.agendum.model.task.UniqueTaskList.TaskNotFoundException; +//@@author A0133367E /** * Deletes task(s) identified using their last displayed indices from the task listing. */ public class DeleteCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "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 + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" + "(The id must be a positive number)\n" + "Example: " + COMMAND_WORD + " 7 10-11"; - public static final String MESSAGE_DELETE_TASK_SUCCESS = "Deleted Task(s): %1$s"; - - public ArrayList targetIndexes; + private ArrayList targetIndexes; + private ArrayList tasksToDelete; - public ArrayList tasksToDelete; - //@@author A0133367E public DeleteCommand(Set targetIndexes) { - this.targetIndexes = new ArrayList<>(targetIndexes); + this.targetIndexes = new ArrayList(targetIndexes); Collections.sort(this.targetIndexes); - this.tasksToDelete = new ArrayList<>(); + this.tasksToDelete = new ArrayList(); } @Override @@ -66,7 +64,6 @@ private boolean isAnyIndexInvalid(UnmodifiableObservableList lastS return targetIndexes.stream().anyMatch(index -> index > lastShownList.size()); } - //@@author public static String getName() { return COMMAND_WORD; } diff --git a/src/main/java/seedu/agendum/logic/commands/MarkCommand.java b/src/main/java/seedu/agendum/logic/commands/MarkCommand.java index e918e751c274..b34e55e32afb 100644 --- a/src/main/java/seedu/agendum/logic/commands/MarkCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/MarkCommand.java @@ -10,33 +10,32 @@ import seedu.agendum.model.task.UniqueTaskList.DuplicateTaskException; import seedu.agendum.model.task.UniqueTaskList.TaskNotFoundException; +//@@author A0133367E /** * Mark task(s) identified using their last displayed indices in the task listing. */ public class MarkCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "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)!"; + public static final String MESSAGE_DUPLICATE = "Hey, the task already exists"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" + "(The id must be a positive number)\n" + "Example: " + COMMAND_WORD + " 1 3 5-6"; - public static final String MESSAGE_MARK_TASK_SUCCESS = "Marked Task(s)!"; - public static final String MESSAGE_DUPLICATE = "Hey, the task already exists"; - - public ArrayList targetIndexes; + private ArrayList targetIndexes; + private ArrayList tasksToMark; - public ArrayList tasksToMark; - //@@author A0133367E public MarkCommand(Set targetIndexes) { - this.targetIndexes = new ArrayList<>(targetIndexes); + this.targetIndexes = new ArrayList(targetIndexes); Collections.sort(this.targetIndexes); - this.tasksToMark = new ArrayList<>(); + this.tasksToMark = new ArrayList(); } @Override @@ -59,7 +58,7 @@ public CommandResult execute() { } catch (TaskNotFoundException pnfe) { assert false : "The target task cannot be missing"; } catch (DuplicateTaskException pnfe) { - model.restoreCurrentToDoListClone(); + model.resetDataToLastSavedList(); return new CommandResult(MESSAGE_DUPLICATE); } @@ -70,7 +69,6 @@ private boolean isAnyIndexInvalid(UnmodifiableObservableList lastS return targetIndexes.stream().anyMatch(index -> index > lastShownList.size()); } - //@@author public static String getName() { return COMMAND_WORD; } diff --git a/src/main/java/seedu/agendum/logic/commands/RenameCommand.java b/src/main/java/seedu/agendum/logic/commands/RenameCommand.java index c8eb30866d79..ae4ed62be2b6 100644 --- a/src/main/java/seedu/agendum/logic/commands/RenameCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/RenameCommand.java @@ -6,34 +6,30 @@ import seedu.agendum.model.task.*; import seedu.agendum.model.task.UniqueTaskList.TaskNotFoundException; - +//@@author A0133367E /** - * Renames a task in the task listing. + * Renames the target task in the task listing. */ public class RenameCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "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"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" + "Example: " + COMMAND_WORD + " 2 Watch Star Trek"; - public static final String MESSAGE_SUCCESS = "Task renamed: %1$s"; - public static final String MESSAGE_DUPLICATE_TASK = "Hey, the task already exists"; - - public int targetIndex = -1; - public Name newTaskName = null; + private int targetIndex; + private Name newTaskName; - //@@author A0133367E /** * Constructor for rename command - * @throws IllegalValueException only if the name is invalid + * @throws IllegalValueException if the name is invalid */ - public RenameCommand(int targetIndex, String name) - throws IllegalValueException { + public RenameCommand(int targetIndex, String name) throws IllegalValueException { this.targetIndex = targetIndex; this.newTaskName = new Name(name); } @@ -64,7 +60,6 @@ public CommandResult execute() { } - //@@author public static String getName() { return COMMAND_WORD; } diff --git a/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java b/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java index 6711743c9f21..c171a1f65f99 100644 --- a/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/ScheduleCommand.java @@ -9,7 +9,7 @@ import seedu.agendum.model.task.*; import seedu.agendum.model.task.UniqueTaskList.TaskNotFoundException; - +//@@author A0003878Y /** * Reschedules a task in the to do list. */ @@ -31,10 +31,9 @@ public class ScheduleCommand extends Command { public static final String MESSAGE_DUPLICATE_TASK = "This task already exists!"; public int targetIndex = -1; - private Optional newStartDateTime = Optional.empty(); - private Optional newEndDateTime = Optional.empty(); + public Optional newStartDateTime = Optional.empty(); + public Optional newEndDateTime = Optional.empty(); - //@@author A0133367E public ScheduleCommand(int targetIndex, Optional startTime, Optional endTime) { Optional balancedEndTime = endTime; @@ -74,7 +73,6 @@ public CommandResult execute() { return new CommandResult(String.format(MESSAGE_SUCCESS, updatedTask)); } - //@author public static String getName() { return 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 4b0fa49d92e3..936e999d7596 100644 --- a/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/UnaliasCommand.java @@ -1,20 +1,19 @@ -//@@author A0133367E package seedu.agendum.logic.commands; import seedu.agendum.model.Model; +//@@author A0133367E /** * Create an alias for a reserved command keyword */ public class UnaliasCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "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_NO_ALIAS_KEY = "The alias <%1$s> does not exist"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" @@ -36,8 +35,7 @@ public void setData(Model model, CommandLibrary commandLibrary) { @Override public CommandResult execute() { if (!commandLibrary.isExistingAliasKey(aliasKey)) { - return new CommandResult(String.format( - MESSAGE_FAILURE_NO_ALIAS_KEY, aliasKey)); + return new CommandResult(String.format(MESSAGE_FAILURE_NO_ALIAS_KEY, aliasKey)); } commandLibrary.removeExistingAlias(aliasKey); diff --git a/src/main/java/seedu/agendum/logic/commands/UndoCommand.java b/src/main/java/seedu/agendum/logic/commands/UndoCommand.java index e7e8fe6483d1..26080a85a77c 100644 --- a/src/main/java/seedu/agendum/logic/commands/UndoCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/UndoCommand.java @@ -1,28 +1,32 @@ package seedu.agendum.logic.commands; +import seedu.agendum.model.ModelManager.NoPreviousListFoundException; + //@@author A0133367E /** - * Undo the last successful command that mutate the to do list + * Undo the last change to the to-do list */ public class UndoCommand extends Command { public static final String COMMAND_WORD = "undo"; - public static final String COMMAND_FORMAT = "undo"; - public static final String COMMAND_DESCRIPTION = "undo the last change to your to-do list"; - - public static final String MESSAGE_SUCCESS = "Previous command undone!"; + public static final String COMMAND_FORMAT = "undo"; + public static final String COMMAND_DESCRIPTION = "undo the last change to your to-do list"; + + public static final String MESSAGE_SUCCESS = "Previous change undone!"; public static final String MESSAGE_FAILURE = "Nothing to undo!"; @Override public CommandResult execute() { assert model != null; - if (model.restorePreviousToDoListClone()) { + + try { + model.restorePreviousToDoList(); return new CommandResult(MESSAGE_SUCCESS); - } else { + } catch (NoPreviousListFoundException nplfe) { return new CommandResult(MESSAGE_FAILURE); } } - + public static String getName() { return COMMAND_WORD; } diff --git a/src/main/java/seedu/agendum/logic/commands/UnmarkCommand.java b/src/main/java/seedu/agendum/logic/commands/UnmarkCommand.java index 8c5fd4ae081e..52803f32dfa2 100644 --- a/src/main/java/seedu/agendum/logic/commands/UnmarkCommand.java +++ b/src/main/java/seedu/agendum/logic/commands/UnmarkCommand.java @@ -10,27 +10,26 @@ import seedu.agendum.model.task.UniqueTaskList.DuplicateTaskException; import seedu.agendum.model.task.UniqueTaskList.TaskNotFoundException; +//@@author A0133367E /** * Unmark task(s) identified using their last displayed indices in the task listing. */ public class UnmarkCommand extends Command { - // COMMAND_WORD, COMMAND_FORMAT, COMMAND_DESCRIPTION are for display in help window public static final String COMMAND_WORD = "unmark"; public static final String COMMAND_FORMAT = "unmark "; public static final String COMMAND_DESCRIPTION = "unmark task(s) from completed"; + + public static final String MESSAGE_UNMARK_TASK_SUCCESS = "Unmarked Task(s)!"; + public static final String MESSAGE_DUPLICATE = "Hey, the task already exists"; public static final String MESSAGE_USAGE = COMMAND_WORD + " - " + COMMAND_DESCRIPTION + "\n" + COMMAND_FORMAT + "\n" + "(The id must be a positive number)\n" + "Example: " + COMMAND_WORD + " 11-13 15"; - public static final String MESSAGE_UNMARK_TASK_SUCCESS = "Unmarked Task(s)!"; - public static final String MESSAGE_DUPLICATE = "Hey, the task already exists"; - - public ArrayList targetIndexes; - - public ArrayList tasksToUnmark; + private ArrayList targetIndexes; + private ArrayList tasksToUnmark; //@@author A0133367E public UnmarkCommand(Set targetIndexes) { @@ -59,7 +58,7 @@ public CommandResult execute() { } catch (TaskNotFoundException pnfe) { assert false : "The target task cannot be missing"; } catch (DuplicateTaskException pnfe) { - model.restoreCurrentToDoListClone(); + model.resetDataToLastSavedList(); return new CommandResult(MESSAGE_DUPLICATE); } @@ -70,7 +69,6 @@ private boolean isAnyIndexInvalid(UnmodifiableObservableList lastS return targetIndexes.stream().anyMatch(index -> index > lastShownList.size()); } - //@@author public static String getName() { return COMMAND_WORD; } diff --git a/src/main/java/seedu/agendum/logic/parser/Parser.java b/src/main/java/seedu/agendum/logic/parser/Parser.java index 0d2db7a686f8..4c4d77f97a0e 100644 --- a/src/main/java/seedu/agendum/logic/parser/Parser.java +++ b/src/main/java/seedu/agendum/logic/parser/Parser.java @@ -77,49 +77,49 @@ public Command parseCommand(String userInput) { switch (commandWord) { - case AddCommand.COMMAND_WORD: + case AddCommand.COMMAND_WORD : return prepareAdd(arguments); - case DeleteCommand.COMMAND_WORD: + case DeleteCommand.COMMAND_WORD : return prepareDelete(arguments); - case FindCommand.COMMAND_WORD: + case FindCommand.COMMAND_WORD : return prepareFind(arguments); - case ListCommand.COMMAND_WORD: + case ListCommand.COMMAND_WORD : return new ListCommand(); - case RenameCommand.COMMAND_WORD: + case RenameCommand.COMMAND_WORD : return prepareRename(arguments); - case MarkCommand.COMMAND_WORD: + case MarkCommand.COMMAND_WORD : return prepareMark(arguments); - case ScheduleCommand.COMMAND_WORD: + case ScheduleCommand.COMMAND_WORD : return prepareSchedule(arguments); - case UnmarkCommand.COMMAND_WORD: + case UnmarkCommand.COMMAND_WORD : return prepareUnmark(arguments); - case UndoCommand.COMMAND_WORD: + case UndoCommand.COMMAND_WORD : return new UndoCommand(); - case AliasCommand.COMMAND_WORD: + case AliasCommand.COMMAND_WORD : return prepareAlias(arguments); - case UnaliasCommand.COMMAND_WORD: + case UnaliasCommand.COMMAND_WORD : return prepareUnalias(arguments); - case ExitCommand.COMMAND_WORD: + case ExitCommand.COMMAND_WORD : return new ExitCommand(); - case HelpCommand.COMMAND_WORD: + case HelpCommand.COMMAND_WORD : return new HelpCommand(); - - case StoreCommand.COMMAND_WORD: + + case StoreCommand.COMMAND_WORD : return new StoreCommand(arguments); - - case LoadCommand.COMMAND_WORD: + + case LoadCommand.COMMAND_WORD : return new LoadCommand(arguments); default: @@ -247,7 +247,7 @@ private void scheduleMatcherOnConsumer(Matcher matcher, BiConsumer index = parseIndex(givenIndex); if (!index.isPresent()) { - return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, RenameCommand.MESSAGE_USAGE)); + return new IncorrectCommand( + String.format(MESSAGE_INVALID_COMMAND_FORMAT, RenameCommand.MESSAGE_USAGE)); } try { @@ -361,7 +363,7 @@ private Command prepareUnalias(String args) { //@@author /** * Returns the specified index in the {@code command} IF a positive unsigned integer is given as the index. - * Returns an {@code Optional.empty()} otherwise. + * Returns an {@code Optional.empty()} otherwise. */ private Optional parseIndex(String command) { final Matcher matcher = TASK_INDEX_ARGS_FORMAT.matcher(command.trim()); @@ -380,14 +382,15 @@ private Optional parseIndex(String command) { //@@author A0133367E /** * Returns the specified indices in the {@code command} if positive unsigned integer(s) are given. - * Returns an empty set otherwise. + * Returns an empty set otherwise. */ private Set parseIndexes(String args) { final Matcher matcher = TASK_INDEXES_ARGS_FORMAT.matcher(args.trim()); - Set taskIds = new HashSet<>(); + Set emptySet = new HashSet(); + Set taskIds = new HashSet(); if (!matcher.matches()) { - return taskIds; + return emptySet; } String replacedArgs = args.replaceAll("[ ]+", ",").replaceAll(",+", ","); @@ -401,18 +404,18 @@ private Set parseIndexes(String args) { int startIndex = Integer.parseInt(startAndEndIndexes[0]); int endIndex = Integer.parseInt(startAndEndIndexes[1]); taskIds.addAll(IntStream.rangeClosed(startIndex, endIndex) - .boxed().collect(Collectors.toList())); + .boxed().collect(Collectors.toList())); } } if (taskIds.remove(0)) { - return new HashSet<>(); + return emptySet; } return taskIds; - } - + } //@@author + /** * Parses arguments in the context of the find task command. * diff --git a/src/main/java/seedu/agendum/model/Model.java b/src/main/java/seedu/agendum/model/Model.java index dbf186a0ad0f..1824e7e749ad 100644 --- a/src/main/java/seedu/agendum/model/Model.java +++ b/src/main/java/seedu/agendum/model/Model.java @@ -2,6 +2,7 @@ import seedu.agendum.commons.core.UnmodifiableObservableList; import seedu.agendum.commons.events.storage.LoadDataCompleteEvent; +import seedu.agendum.model.ModelManager.NoPreviousListFoundException; import seedu.agendum.model.task.ReadOnlyTask; import seedu.agendum.model.task.Task; import seedu.agendum.model.task.UniqueTaskList; @@ -38,15 +39,14 @@ void unmarkTasks(List targets) throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException; /** - * Restores the previous (second latest) to do list saved in the event of an undo operation - * Returns true if successful; false if there are no earlier lists + * Restores the previous to-do list saved in the stack of previous lists. */ - boolean restorePreviousToDoListClone(); + void restorePreviousToDoList() throws NoPreviousListFoundException; /** - * Restores the current (latest) to do list saved in the event of exceptions + * Resets the to-do list data to match the top list in the stack of previous lists */ - void restoreCurrentToDoListClone(); + void resetDataToLastSavedList(); /** Returns the filtered task list as an {@code UnmodifiableObservableList} */ UnmodifiableObservableList getFilteredTaskList(); diff --git a/src/main/java/seedu/agendum/model/ModelManager.java b/src/main/java/seedu/agendum/model/ModelManager.java index 4dfd458111b2..d1adf514150f 100644 --- a/src/main/java/seedu/agendum/model/ModelManager.java +++ b/src/main/java/seedu/agendum/model/ModelManager.java @@ -35,6 +35,14 @@ public class ModelManager extends ComponentManager implements Model { private final FilteredList filteredTasks; private final SortedList sortedTasks; + //@@author A0133367E + /** + * Signals that an operation to remove a list from the stack of previous lists would fail + * as the stack must contain at least one list. + */ + public static class NoPreviousListFoundException extends Exception {} + //@@author + /** * Initializes a ModelManager with the given ToDoList * ToDoList and its variables should not be null @@ -49,7 +57,7 @@ public ModelManager(ToDoList src, UserPrefs userPrefs) { toDoList = new ToDoList(src); filteredTasks = new FilteredList<>(toDoList.getTasks()); sortedTasks = filteredTasks.sorted(); - previousLists = new Stack<>(); + previousLists = new Stack(); backupCurrentToDoList(); } @@ -59,9 +67,9 @@ public ModelManager() { public ModelManager(ReadOnlyToDoList initialData) { toDoList = new ToDoList(initialData); - filteredTasks = new FilteredList<>(toDoList.getTasks()); + filteredTasks = new FilteredList(toDoList.getTasks()); sortedTasks = filteredTasks.sorted(); - previousLists = new Stack<>(); + previousLists = new Stack(); backupCurrentToDoList(); } @@ -73,8 +81,8 @@ public void resetData(ReadOnlyToDoList newData) { backupCurrentToDoList(); indicateToDoListChanged(); } - //@@author + @Override public ReadOnlyToDoList getToDoList() { return toDoList; @@ -104,14 +112,16 @@ public synchronized void deleteTasks(List targets) throws TaskNotF for (ReadOnlyTask target: targets) { toDoList.removeTask(target); } - backupCurrentToDoList(); + logger.fine("[MODEL] --- successfully deleted all specified targets from the to-do list"); + backupCurrentToDoList(); indicateToDoListChanged(); } @Override public synchronized void addTask(Task task) throws UniqueTaskList.DuplicateTaskException { - toDoList.addTask(task); + toDoList.addTask(task); + logger.fine("[MODEL] --- successfully added the new task to the to-do list"); backupCurrentToDoList(); updateFilteredListToShowAll(); @@ -122,6 +132,7 @@ public synchronized void addTask(Task task) throws UniqueTaskList.DuplicateTaskE public synchronized void updateTask(ReadOnlyTask target, Task updatedTask) throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { toDoList.updateTask(target, updatedTask); + logger.fine("[MODEL] --- successfully updated the target task in the to-do list"); backupCurrentToDoList(); updateFilteredListToShowAll(); @@ -134,6 +145,7 @@ public synchronized void markTasks(List targets) for (ReadOnlyTask target: targets) { toDoList.markTask(target); } + logger.fine("[MODEL] --- successfully marked all specified targets from the to-do list"); backupCurrentToDoList(); indicateToDoListChanged(); @@ -145,42 +157,31 @@ public synchronized void unmarkTasks(List targets) for (ReadOnlyTask target: targets) { toDoList.unmarkTask(target); } + logger.fine("[MODEL] --- successfully unmarked all specified targets from the to-do list"); backupCurrentToDoList(); indicateToDoListChanged(); } /** - * This is to restore the previous (second latest) list saved - * in the event of an "undo" operation + * Restores the previous (second latest) list saved in the stack of previous lists. */ @Override - public synchronized boolean restorePreviousToDoListClone() { - assert !previousLists.empty(); - - if (previousLists.size() == 1) { - return false; - } else { - previousLists.pop(); - toDoList.resetData(previousLists.peek()); - logger.fine("[MODEL] --- successfully restored the previous the to-do list from this session"); - indicateToDoListChanged(); - return true; - } + public synchronized void restorePreviousToDoList() throws NoPreviousListFoundException { + removeLastSavedToDoList(); + resetDataToLastSavedList(); + logger.fine("[MODEL] --- successfully restored the previous the to-do list from this session"); + indicateToDoListChanged(); } /** - * This is to reverse any temporary changes to the to-do list - * that have not been saved to storage or stack of previous lists (in the event of exceptions) + * Resets the to-do list data to match the top list in the stack of previous lists */ @Override - public synchronized void restoreCurrentToDoListClone() { + public void resetDataToLastSavedList() { assert !previousLists.empty(); - - logger.fine("[MODEL] --- successfully restored the current to-do list" - + " before exceptions/temporary changes"); - - toDoList.resetData(previousLists.peek()); + ToDoList lastSavedListFromHistory = previousLists.peek(); + toDoList.resetData(lastSavedListFromHistory); } private void backupCurrentToDoList() { @@ -192,10 +193,24 @@ private void clearAllPreviousToDoLists() { previousLists.clear(); } + /** + * Pops the top list from the stack of previous lists if there are more than 1 list in the stack + * @throws NoPreviousListFoundException if there is only 1 list in the stack + */ + private void removeLastSavedToDoList() throws NoPreviousListFoundException { + assert !previousLists.empty(); + + if (previousLists.size() == 1) { + throw new NoPreviousListFoundException(); + } + + previousLists.pop(); + } + - //=========== Storage Methods ========================================================================== - //@@author A0148095X + //=========== Storage Methods ========================================================================== + @Override public synchronized void changeSaveLocation(String location){ assert StringUtil.isValidPathToFile(location); diff --git a/src/main/java/seedu/agendum/model/ToDoList.java b/src/main/java/seedu/agendum/model/ToDoList.java index 128bfe49495e..6a66a2253896 100644 --- a/src/main/java/seedu/agendum/model/ToDoList.java +++ b/src/main/java/seedu/agendum/model/ToDoList.java @@ -78,34 +78,34 @@ public boolean removeTask(ReadOnlyTask key) throws TaskNotFoundException { //@@author A0133367E /** * Updates an existing task in the to-do list. + * * @throws DuplicateTaskException if an equivalent task (to updatedTask) already exists. * @throws TaskNotFoundException if no such task (key) could be found in the list. */ public boolean updateTask(ReadOnlyTask key, Task updatedTask) - throws TaskNotFoundException, - DuplicateTaskException { + throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { return tasks.update(key, updatedTask); } /** * Marks an existing task in the to-do list. + * * @throws DuplicateTaskException if a duplicate task would result after marking key. * @throws TaskNotFoundException if no such task (key) could be found in the list. */ public boolean markTask(ReadOnlyTask key) - throws TaskNotFoundException, - DuplicateTaskException { + throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { return tasks.mark(key); } /** * Unmarks an existing task in the to-do list. + * * @throws DuplicateTaskException if a duplicate task would result after unmarking key. * @throws TaskNotFoundException if no such task (key) could be found in the list. */ - public boolean unmarkTask(ReadOnlyTask key) - throws TaskNotFoundException, - DuplicateTaskException { + public boolean unmarkTask(ReadOnlyTask key) + throws UniqueTaskList.TaskNotFoundException, UniqueTaskList.DuplicateTaskException { return tasks.unmark(key); } //@@author diff --git a/src/main/java/seedu/agendum/model/task/ReadOnlyTask.java b/src/main/java/seedu/agendum/model/task/ReadOnlyTask.java index 815b89d9debe..9af3b7de8c05 100644 --- a/src/main/java/seedu/agendum/model/task/ReadOnlyTask.java +++ b/src/main/java/seedu/agendum/model/task/ReadOnlyTask.java @@ -36,29 +36,34 @@ default boolean isSameStateAs(ReadOnlyTask other) { * Formats the task as text, showing task name only. */ default String getAsText() { - return String.valueOf(getName()) + - "\n"; + return String.valueOf(getName()) + "\n"; } //@@author A0133367E /** - * Format the tasks as text, showing all details including name, - * completion status, start and end time if any + * Format the tasks as text, showing all fine details including name, + * completion status, start and end time if any and last updated time */ default String getDetailedText() { - String completionStatus = isCompleted() ? "Completed" : "Incomplete"; - String startTime = getStartDateTime().isPresent() ? getStartDateTime().get().toString() : "None"; - String endTime = getEndDateTime().isPresent() ? getEndDateTime().get().toString() : "None"; + String completionStatus = (isCompleted()) ? "Completed" : "Incomplete"; + String startTime = (getStartDateTime().isPresent()) ? getStartDateTime().get().toString() + : "None"; + String endTime = (getEndDateTime().isPresent()) ? getEndDateTime().get().toString() + : "None"; + String lastUpdatedTime = getLastUpdatedTime().toString(); - return "Task name: " + - getName() + - " Status: " + - completionStatus + - " Start Time: " + - startTime + - " End Time: " + - endTime + - " Last Updated Time: " + - getLastUpdatedTime().toString(); + final StringBuilder builder = new StringBuilder(); + builder.append("Task name: ") + .append(getName()) + .append(" Completion Status: ") + .append(completionStatus) + .append(" Start Time: ") + .append(startTime) + .append(" End Time: ") + .append(endTime) + .append(" Last Updated Time: ") + .append(lastUpdatedTime); + return builder.toString(); } + } diff --git a/src/main/java/seedu/agendum/model/task/Task.java b/src/main/java/seedu/agendum/model/task/Task.java index 2029374cfca3..498e62a90d1c 100644 --- a/src/main/java/seedu/agendum/model/task/Task.java +++ b/src/main/java/seedu/agendum/model/task/Task.java @@ -3,7 +3,6 @@ import seedu.agendum.commons.util.CollectionUtil; import java.time.LocalDateTime; -import java.time.temporal.ChronoUnit; import java.util.Objects; import java.util.Optional; @@ -15,11 +14,11 @@ public class Task implements ReadOnlyTask, Comparable { private static final int UPCOMING_DAYS_THRESHOLD = 7; - private Name name; - private boolean isCompleted; - private LocalDateTime startDateTime; - private LocalDateTime endDateTime; - private LocalDateTime lastUpdatedTime; + private Name name_; + private boolean isCompleted_; + private LocalDateTime startDateTime_; + private LocalDateTime endDateTime_; + private LocalDateTime lastUpdatedTime_; // ================ Constructor methods ============================== @@ -28,10 +27,10 @@ public class Task implements ReadOnlyTask, Comparable { */ public Task(Name name) { assert CollectionUtil.isNotNull(name); - this.name = name; - this.isCompleted = false; - this.startDateTime = null; - this.endDateTime = null; + this.name_ = name; + this.isCompleted_ = false; + this.startDateTime_ = null; + this.endDateTime_ = null; setLastUpdatedTimeToNow(); } @@ -40,11 +39,11 @@ public Task(Name name) { */ public Task(Name name, Optional deadline) { assert CollectionUtil.isNotNull(name); - this.name = name; - this.isCompleted = false; - this.startDateTime = null; - this.endDateTime = deadline.orElse(null); - setLastUpdatedTimeToNow(); + this.name_ = name; + this.isCompleted_ = false; + this.startDateTime_ = null; + this.endDateTime_ = deadline.orElse(null); + this.setLastUpdatedTimeToNow(); } /** @@ -53,11 +52,11 @@ public Task(Name name, Optional deadline) { public Task(Name name, Optional startDateTime, Optional endDateTime) { assert CollectionUtil.isNotNull(name); - this.name = name; - this.isCompleted = false; - this.startDateTime = startDateTime.orElse(null); - this.endDateTime = endDateTime.orElse(null); - setLastUpdatedTimeToNow(); + this.name_ = name; + this.isCompleted_ = false; + this.startDateTime_ = startDateTime.orElse(null); + this.endDateTime_ = endDateTime.orElse(null); + this.setLastUpdatedTimeToNow(); } /** @@ -68,43 +67,83 @@ public Task(ReadOnlyTask source) { if (source.isCompleted()) { this.markAsCompleted(); } - setLastUpdatedTime(source.getLastUpdatedTime()); + this.setLastUpdatedTime(source.getLastUpdatedTime()); } // ================ Getter methods ============================== @Override public Name getName() { - return name; + return name_; } @Override public boolean isCompleted() { - return isCompleted; + return isCompleted_; } + /** + * Returns true if a task is uncompleted and has a start/end time + * that is after the current time but within some threshold amount of days + */ @Override public boolean isUpcoming() { - return !isCompleted() && hasTime() && !isOverdue() - && getTaskTime().isBefore(LocalDateTime.now().plusDays(UPCOMING_DAYS_THRESHOLD)); + if (isCompleted()) { + return false; + } + + if (!hasTime()) { + return false; + } + + LocalDateTime currentTime = LocalDateTime.now(); + LocalDateTime thresholdTime = currentTime.plusDays(UPCOMING_DAYS_THRESHOLD); + boolean isBeforeUpcomingDaysThreshold = getTaskTime().isBefore(thresholdTime); + boolean isAfterCurrentTime = getTaskTime().isAfter(currentTime); + + return isBeforeUpcomingDaysThreshold && isAfterCurrentTime; } + /** + * Returns true is a task is uncompleted and has a start/end time + * that is before the current time + */ @Override public boolean isOverdue() { - return !isCompleted() && hasTime() - && getTaskTime().isBefore(LocalDateTime.now()); + if (isCompleted()) { + return false; + } + + if (!hasTime()) { + return false; + } + + LocalDateTime currentTime = LocalDateTime.now(); + boolean isBeforeCurrentTime = getTaskTime().isBefore(currentTime); + + return isBeforeCurrentTime; } + /** + * Returns true if a task has a start time or an end time, false otherwise + * This must be called to check if comparison of task's time is possible + */ @Override public boolean hasTime() { - return (getStartDateTime().isPresent() || getEndDateTime().isPresent()); + return getStartDateTime().isPresent() || getEndDateTime().isPresent(); } + /** + * Returns true if the task has a start time and end time, false otherwise. + */ @Override public boolean isEvent() { - return getStartDateTime().isPresent(); + return getStartDateTime().isPresent() && getEndDateTime().isPresent(); } + /** + * Returns true if the task has a deadline (i.e. only a end time), false otherwise. + */ @Override public boolean hasDeadline() { return !getStartDateTime().isPresent() && getEndDateTime().isPresent(); @@ -112,22 +151,26 @@ public boolean hasDeadline() { @Override public Optional getStartDateTime() { - return Optional.ofNullable(startDateTime); + return Optional.ofNullable(startDateTime_); } @Override public Optional getEndDateTime() { - return Optional.ofNullable(endDateTime); + return Optional.ofNullable(endDateTime_); } + /** + * Returns the time the task is last updated. + * e.g. created, renamed, rescheduled, marked or unmarked + */ @Override public LocalDateTime getLastUpdatedTime() { - return lastUpdatedTime; + return lastUpdatedTime_; } /** - * Pre-condition: Task has a start or end time - * Return the (earlier) time associated with the task (assumed to be start time) + * Pre-condition: Task has a start or end time. + * Returns the start time if present, else returns the end time. */ private LocalDateTime getTaskTime() { assert hasTime(); @@ -137,36 +180,37 @@ private LocalDateTime getTaskTime() { // ================ Setter methods ============================== public void setName(Name name) { - this.name = name; + this.name_ = name; setLastUpdatedTimeToNow(); } public void markAsCompleted() { - this.isCompleted = true; + this.isCompleted_ = true; setLastUpdatedTimeToNow(); } public void markAsUncompleted() { - this.isCompleted = false; + this.isCompleted_ = false; setLastUpdatedTimeToNow(); } public void setStartDateTime(Optional startDateTime) { - this.startDateTime = startDateTime.orElse(null); + this.startDateTime_ = startDateTime.orElse(null); setLastUpdatedTimeToNow(); } public void setEndDateTime(Optional endDateTime) { - this.endDateTime = endDateTime.orElse(null); + this.endDateTime_ = endDateTime.orElse(null); setLastUpdatedTimeToNow(); } public void setLastUpdatedTime(LocalDateTime updatedTime) { - this.lastUpdatedTime = updatedTime; + this.lastUpdatedTime_ = updatedTime; } public void setLastUpdatedTimeToNow() { - this.lastUpdatedTime = LocalDateTime.now().withNano(0); + // nano-seconds is set to 0 for more consistent test results when (un)marking multiple tasks + this.lastUpdatedTime_ = LocalDateTime.now().withNano(0); } // ================ Other methods ============================== @@ -178,6 +222,15 @@ public boolean equals(Object other) { && this.isSameStateAs((ReadOnlyTask) other)); } + /** + * Compares the current task with another Task other. + * The current task is considered to be less than the other task if + * 1) it is uncompleted and other is completed + * 2) both tasks are completed but this task has a earlier start/end time associated + * 3) both tasks are uncompleted but this task has a later updated time + * 4) both tasks are uncompleted with the same updated time + * but this task has a lexicographically smaller name (useful when sorting tasks in testing) + */ @Override public int compareTo(Task other) { int comparedCompletionStatus = compareCompletionStatus(other); @@ -198,10 +251,23 @@ public int compareTo(Task other) { return compareName(other); } + /** + * Compares the completion status of current task with another Task other. + * The current task is considered to be less than the other task if + * it is uncompleted and other is completed + */ public int compareCompletionStatus(Task other) { return Boolean.compare(this.isCompleted(), other.isCompleted()); } + /** + * Compares the earliest time of the current task with another Task other. + * The current task is considered to be less than the other task if + * 1) both tasks have a time associated but this task has a earlier time associated + * 2) this task has a time associated but the other task does not. + * Both tasks are equal if they have no time or the same earliest time associated. + * Time refers to value returned by {@link #getTaskTime()} + */ public int compareTaskTime(Task other) { if (this.hasTime() && other.hasTime()) { return this.getTaskTime().compareTo(other.getTaskTime()); @@ -214,10 +280,20 @@ public int compareTaskTime(Task other) { } } + /** + * Compares the current task with another Task other. + * The current task is considered to be less than the other task if + * it has a later updated time + */ public int compareLastUpdatedTime(Task other) { return other.getLastUpdatedTime().compareTo(this.getLastUpdatedTime()); } + /** + * Compares the current task with another Task other. + * The current task is considered to be less than the other task if + * it has a lexicographically smaller name + */ public int compareName(Task other) { return this.getName().toString().compareTo(other.getName().toString()); } @@ -225,7 +301,7 @@ public int compareName(Task other) { @Override public int hashCode() { // use this method for custom fields hashing instead of implementing your own - return Objects.hash(name, isCompleted, startDateTime, endDateTime); + return Objects.hash(name_, isCompleted_, startDateTime_, endDateTime_); } @Override diff --git a/src/main/java/seedu/agendum/storage/AliasTableStorage.java b/src/main/java/seedu/agendum/storage/AliasTableStorage.java index 2d6cbd231a9d..c9c7aff92dd4 100644 --- a/src/main/java/seedu/agendum/storage/AliasTableStorage.java +++ b/src/main/java/seedu/agendum/storage/AliasTableStorage.java @@ -12,7 +12,7 @@ public interface AliasTableStorage { /** - * Returns the alias table (map of alias key to original command) from storage. + * Returns the alias table (map of alias key to reserved command word) from storage. * Returns {@code Optional.empty()} if storage file is not found. * @throws DataConversionException if the data in storage is not in the expected format. * @throws IOException if there was any problem when reading from the storage. @@ -21,7 +21,7 @@ Optional> readAliasTable() throws DataConversionException, IOException; /** - * Saves the alias table in {{@link seedu.agendum.logic.commands.CommandLibrary} to the storage. + * Saves the alias table from {@link seedu.agendum.logic.commands.CommandLibrary} to the storage. * @param table cannot be null. * @throws IOException if there was any problem writing to the file. */ diff --git a/src/main/java/seedu/agendum/storage/JsonAliasTableStorage.java b/src/main/java/seedu/agendum/storage/JsonAliasTableStorage.java index 7854561b1b7b..6a4d424e84af 100644 --- a/src/main/java/seedu/agendum/storage/JsonAliasTableStorage.java +++ b/src/main/java/seedu/agendum/storage/JsonAliasTableStorage.java @@ -3,7 +3,6 @@ import seedu.agendum.commons.core.LogsCenter; import seedu.agendum.commons.exceptions.DataConversionException; import seedu.agendum.commons.util.FileUtil; -import seedu.agendum.model.UserPrefs; import java.io.File; import java.io.IOException; @@ -63,7 +62,7 @@ public Optional> readAliasTable(String aliasTableFileP } /** - * Similar to {@link #saveAliasTable(Hashtable table)} + * Similar to {@link #saveAliasTable(Hashtable)} * @param aliasTableFilePath location of the command library's alias table data. Cannot be null. */ public void saveAliasTable(Hashtable table, String aliasTableFilePath) diff --git a/src/test/java/guitests/AddCommandTest.java b/src/test/java/guitests/AddCommandTest.java index a1b47e23a356..e752899a8af8 100644 --- a/src/test/java/guitests/AddCommandTest.java +++ b/src/test/java/guitests/AddCommandTest.java @@ -10,6 +10,7 @@ import seedu.agendum.testutil.TestUtil; import seedu.agendum.testutil.TypicalTestTasks; +//@@author A0133367E public class AddCommandTest extends ToDoListGuiTest { @Test @@ -48,15 +49,15 @@ private void assertAddSuccess(TestTask taskToAdd, TestTask... currentList) { commandBox.runCommand(taskToAdd.getAddCommand()); //confirm the new card contains the right data - if (!taskToAdd.isCompleted() && !taskToAdd.hasTime()) { + if (taskToAdd.isCompleted()) { + TaskCardHandle addedCard = completedTasksPanel.navigateToTask(taskToAdd.getName().fullName); + assertMatching(taskToAdd, addedCard); + } else if (!taskToAdd.isCompleted() && !taskToAdd.hasTime()) { TaskCardHandle addedCard = floatingTasksPanel.navigateToTask(taskToAdd.getName().fullName); assertMatching(taskToAdd, addedCard); } else if (!taskToAdd.isCompleted() && taskToAdd.hasTime()) { TaskCardHandle addedCard = upcomingTasksPanel.navigateToTask(taskToAdd.getName().fullName); assertMatching(taskToAdd, addedCard); - } else { - TaskCardHandle addedCard = completedTasksPanel.navigateToTask(taskToAdd.getName().fullName); - assertMatching(taskToAdd, addedCard); } //confirm the list now contains all previous tasks plus the new task diff --git a/src/test/java/guitests/ToDoListGuiTest.java b/src/test/java/guitests/ToDoListGuiTest.java index 3389f1645c10..82738e7799fb 100644 --- a/src/test/java/guitests/ToDoListGuiTest.java +++ b/src/test/java/guitests/ToDoListGuiTest.java @@ -130,10 +130,10 @@ protected void assertResultMessage(String expected) { protected void assertShowingMessage(String expected) { assertEquals(expected, messageDisplay.getText()); } - + + //@@author A0133367E /** - * expectedList is a sorted list of all tasks. - * Asserts the task shown in each panel will match + * Asserts the tasks shown in each panel will match */ protected void assertAllPanelsMatch(TestTask[] expectedList) { TestUtil.sortTasks(expectedList); diff --git a/src/test/java/seedu/agendum/logic/LogicManagerTest.java b/src/test/java/seedu/agendum/logic/LogicManagerTest.java index ae9e2c545c80..7fa367635df5 100644 --- a/src/test/java/seedu/agendum/logic/LogicManagerTest.java +++ b/src/test/java/seedu/agendum/logic/LogicManagerTest.java @@ -288,9 +288,8 @@ private void assertIndexNotFoundBehaviorForCommand(String commandWord, String wo // set AB state to 2 tasks model.resetData(new ToDoList()); - for (Task p : taskList) { - model.addTask(p); - } + helper.addToModel(model, taskList); + // test boundary value (one-based index is 3 when list is of size 2) assertCommandBehavior(commandWord + " 3 " + wordsAfterIndex, MESSAGE_INVALID_TASK_DISPLAYED_INDEX, model.getToDoList(), taskList); } @@ -312,9 +311,8 @@ private void assertIndexNotFoundBehaviorForCommand(String commandWord) throws Ex // set AB state to 5 tasks model.resetData(new ToDoList()); - for (Task p : taskList) { - model.addTask(p); - } + helper.addToModel(model, taskList); + // test boundary value (one-based index is 6 when list is of size 5) //invalid index is the last index given assertCommandBehavior(commandWord + " 1 6", expectedMessage, model.getToDoList(), taskList); @@ -350,7 +348,7 @@ public void execute_delete_removesCorrectSingleTask() throws Exception { // prepare model helper.addToModel(model, threeTasks); - // prepare for message + // prepare message List deletedTaskVisibleIndices = helper.generateNumberList(3); List deletedTasks = helper.generateReadOnlyTaskList(threeTasks.get(2)); String tasksAsString = CommandResult.tasksToString(deletedTasks, deletedTaskVisibleIndices); @@ -375,7 +373,7 @@ public void execute_delete_removesCorrectRangeOfTasks() throws Exception { // prepare model helper.addToModel(model, fourTasks); - //prepare for message + //prepare message List deletedTaskVisibleIndices = helper.generateNumberList(2, 3); List deletedTasks = helper.generateReadOnlyTaskList(fourTasks.get(1), fourTasks.get(2)); String tasksAsString = CommandResult.tasksToString(deletedTasks, deletedTaskVisibleIndices); @@ -402,7 +400,7 @@ public void execute_delete_removesCorrectMultipleTasks() throws Exception { // prepare model helper.addToModel(model, fourTasks); - // prepare for message + // prepare message List deletedTaskVisibleIndices = helper.generateNumberList(2, 3, 4); List deletedTasks = helper.generateReadOnlyTaskList( fourTasks.get(1), fourTasks.get(2), fourTasks.get(3)); @@ -656,7 +654,7 @@ public void execute_renameInvalidArgsFormat_errorMessageShown() throws Exception // invalid index format // a valid name is provided since invalid input values must be tested one at a time String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, RenameCommand.MESSAGE_USAGE); - assertIncorrectIndexFormatBehaviorForCommand("rename", expectedMessage, "new task name"); + assertIncorrectIndexFormatBehaviorForCommand("rename", expectedMessage, " new task name"); // invalid new task name format e.g. task name is not provided TestDataHelper helper = new TestDataHelper(); @@ -672,7 +670,7 @@ public void execute_renameInvalidArgsFormat_errorMessageShown() throws Exception @Test public void execute_renameIndexNotFound_errorMessageShown() throws Exception { // a valid name is provided to only test for invalid index - assertIndexNotFoundBehaviorForCommand("rename", "new task name"); + assertIndexNotFoundBehaviorForCommand("rename", " new task name"); } @Test @@ -711,7 +709,7 @@ public void execute_rename_RenamesCorrectTask() throws Exception { // prepare model helper.addToModel(model, threeTasks); - //boundary value: use the last task + // boundary value: use the last task assertCommandBehavior("rename 3 " + newTaskName, String.format(RenameCommand.MESSAGE_SUCCESS, newTaskName), expectedTDL, @@ -788,7 +786,7 @@ public void execute_schedule_scheduleCorrectTask() throws Exception { ToDoList expectedTDL = helper.generateToDoList(threeTasks); expectedTDL.updateTask(floatingTask, eventTask); - //prepare model + // prepare model helper.addToModel(model, threeTasks); assertCommandBehavior("schedule 3 from Sep 9 9:10 to Oct 10 10:10", @@ -802,7 +800,7 @@ public void execute_schedule_scheduleCorrectTask() throws Exception { public void execute_aliasInvalidArgsFormat_errorMessageShown() throws Exception { String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, AliasCommand.MESSAGE_USAGE); assertCommandBehavior("alias", expectedMessage, new ToDoList(), Collections.emptyList()); - //alias should not contain symbols + // alias should not contain symbols assertCommandBehavior("alias add +", expectedMessage, new ToDoList(), Collections.emptyList()); // new alias key has space assertCommandBehavior("alias add a 1", expectedMessage, new ToDoList(), Collections.emptyList()); @@ -818,8 +816,8 @@ public void execute_aliasNonOriginalCommand_errorMessageShown() throws Exception public void execute_aliasKeyIsReservedCommandWord_errorMessageShown() throws Exception { String expectedMessage = String.format(AliasCommand.MESSAGE_FAILURE_UNAVAILABLE_ALIAS, RenameCommand.COMMAND_WORD); - assertCommandBehavior("alias " + AddCommand.COMMAND_WORD + " " + RenameCommand.COMMAND_WORD, - expectedMessage, new ToDoList(), Collections.emptyList()); + String userCommand = "alias " + AddCommand.COMMAND_WORD + " " + RenameCommand.COMMAND_WORD; + assertCommandBehavior(userCommand, expectedMessage, new ToDoList(), Collections.emptyList()); } @Test @@ -827,16 +825,19 @@ public void execute_aliasKeyIsInUse_errorMessageShown() throws Exception { String expectedMessage = String.format(AliasCommand.MESSAGE_FAILURE_ALIAS_IN_USE, "r", RenameCommand.COMMAND_WORD); CommandLibrary.getInstance().addNewAlias("r", RenameCommand.COMMAND_WORD); - assertCommandBehavior("alias " + AddCommand.COMMAND_WORD + " r", - expectedMessage, new ToDoList(), Collections.emptyList()); + String userCommand = "alias " + AddCommand.COMMAND_WORD + " r"; + assertCommandBehavior(userCommand, expectedMessage, new ToDoList(), Collections.emptyList()); } @Test - public void execute_aliasValidKey_successfullyAdded() throws Exception { + public void execute_aliasValidCaseInsensitiveKey_successfullyAdded() throws Exception { + // successfully add alias String expectedMessage = String.format(AliasCommand.MESSAGE_SUCCESS, "a", AddCommand.COMMAND_WORD); - //should be case insensitive + assertCommandBehavior("alias " + AddCommand.COMMAND_WORD + " A", expectedMessage, new ToDoList(), Collections.emptyList()); + + // alias can be used TestDataHelper helper = new TestDataHelper(); Task addedTask = helper.generateTaskWithName("new task"); @@ -864,9 +865,12 @@ public void execute_unaliasNoSuchKey_errorMessageShown() throws Exception { @Test public void execute_unaliasExistingAliasKey_successfullyRemoved() throws Exception { + // successfully remove alias String expectedMessage = String.format(UnaliasCommand.MESSAGE_SUCCESS, "wow"); CommandLibrary.getInstance().addNewAlias("wow", AddCommand.COMMAND_WORD); assertCommandBehavior("unalias wow", expectedMessage, new ToDoList(), Collections.emptyList()); + + // previous alias key cannot be used expectedMessage = MESSAGE_UNKNOWN_COMMAND; assertCommandBehavior("wow new task", expectedMessage, new ToDoList(), Collections.emptyList()); } @@ -937,42 +941,42 @@ public void executeFindMatchesIfAnyKeywordPresent() throws Exception { //@@author A0133367E @Test - public void execute_undo_identifiesNoPreviousCommand() throws Exception { + public void execute_undo_identifiesNoPreviousChanges() throws Exception { assertCommandBehavior("undo", UndoCommand.MESSAGE_FAILURE, new ToDoList(), Collections.emptyList()); } @Test - public void execute_undo_reversePreviousMutatingCommand() throws Exception { + public void execute_undo_reversePreviousChangeToTaskList() throws Exception { TestDataHelper helper = new TestDataHelper(); Task p1 = helper.generateTaskWithName("old name"); List listWithOneTask = helper.generateTaskList(p1); ToDoList expectedTDL = helper.generateToDoList(listWithOneTask); List readOnlyTaskList = helper.generateReadOnlyTaskList(p1); - //Undo add command + // Undo add command model.addTask(p1); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, new ToDoList(), Collections.emptyList()); - //Undo delete command + // Undo delete command model.addTask(p1); model.deleteTasks(readOnlyTaskList); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - //Undo clear command + // Undo clear command model.resetData(new ToDoList()); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - //Undo rename command + // Undo rename command Task p2 = new Task(p1); p2.setName(new Name("new name")); model.updateTask(p1, p2); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - //Undo mark command + // Undo mark command model.markTasks(readOnlyTaskList); assertCommandBehavior("undo", UndoCommand.MESSAGE_SUCCESS, expectedTDL, listWithOneTask); - //Undo unmark command + // Undo unmark command model.markTasks(readOnlyTaskList); Task p3 = new Task(p1); //p1 clone p3.markAsCompleted(); diff --git a/src/test/java/seedu/agendum/model/TaskTest.java b/src/test/java/seedu/agendum/model/TaskTest.java index aca5332be8e1..6aaeaad87da8 100644 --- a/src/test/java/seedu/agendum/model/TaskTest.java +++ b/src/test/java/seedu/agendum/model/TaskTest.java @@ -89,7 +89,7 @@ public void isUpcoming_taskWithEndTimeYesterday_returnsFalse() { } @Test - public void isUpcoming_uncompletedTaskFromNextYear_returnsFalse() { + public void isUpcoming_uncompletedTaskFromNextMonth_returnsFalse() { LocalDateTime nextMonth = LocalDateTime.now().plusMonths(1); floatingTask.setEndDateTime(Optional.ofNullable(nextMonth)); assertFalse(floatingTask.isUpcoming()); @@ -201,7 +201,7 @@ public void equals_tasksWithOnlyDifferentTaskTime_returnsFalse() throws Exceptio } @Test - public void equals_tasksWithOnlyDifferentUpdatedTme_returnsTrue() throws Exception { + public void equals_tasksWithOnlyDifferentUpdatedTime_returnsTrue() throws Exception { Task copiedTask = new Task(floatingTask); copiedTask.setLastUpdatedTimeToNow(); assertEquals(floatingTask, copiedTask); @@ -233,11 +233,12 @@ public void compareTo_uncompletedTasks_earlierAndPresentTaskTimeFirst() { * (Regardless of the start and end time associated with the task) */ @Test - public void compareTo_completedTasks_earlierUpdatedTimeFirst() { + public void compareTo_completedTasks_laterUpdatedTimeFirst() { deadlineTaskDueTomorrow.markAsCompleted(); deadlineTaskDueTomorrow.setLastUpdatedTime(yesterday.get()); + // have later updated time deadlineTaskDueYesterday.markAsCompleted(); deadlineTaskDueYesterday.setLastUpdatedTime(tomorrow.get()); diff --git a/src/test/java/seedu/agendum/model/UniqueTaskListTest.java b/src/test/java/seedu/agendum/model/UniqueTaskListTest.java index 9c071ef07e55..1dfc4b037fc8 100644 --- a/src/test/java/seedu/agendum/model/UniqueTaskListTest.java +++ b/src/test/java/seedu/agendum/model/UniqueTaskListTest.java @@ -29,10 +29,13 @@ public class UniqueTaskListTest { @Before public void setUp() throws IllegalValueException { uniqueTaskList = new UniqueTaskList(); + originalTask = new Task(new Name("task")); duplicateOfOriginalTask = new Task(originalTask); newTask = new Task(new Name("new task")); + uniqueTaskList.add(originalTask); + internalList = FXCollections.observableArrayList(); internalList.add(originalTask); }