Merge pull request #116 from Esilocke/findPriority

[V1.5] Improving Find command functionality

docs/UserGuide.adoc

@@ -285,8 +285,9 @@ Returns any person having names `Betsy`, `Tim`, or `John`
 
 Adding `task` after `find` will allow you to sieve through your tasks, instead of your contacts. +
 In addition to searching the name and description of tasks, you can also opt to filter your tasks by their priority. Simply include `p/PRIORITY` after all your other criteria to do so. +
+You can also opt to find all tasks that are either complete or incomplete by including an optional `done/ISTASKDONE` in the command.
 All tasks with a priority higher than or equal to the value provided will be shown. +
-Format: `find task KEYWORD [MORE_KEYWORDS] [p/PRIORITY]` (Priority search to be coming in v2.0) +
+Format: `find task KEYWORD [MORE_KEYWORDS] [p/PRIORITY] [done/ISTASKDONE]` +
 
 ****
 *Important note on `find` criteria*
@@ -296,6 +297,9 @@ Format: `find task KEYWORD [MORE_KEYWORDS] [p/PRIORITY]` (Priority search to be
 * You can only search for names in Address++
 * Only full words will be matched e.g. `Han` will not match `Hans`
 * Persons matching at least one keyword will be returned (i.e. `OR` search). e.g. `Hans Bo` will return `Hans Gruber`, `Bo Yang`
+* *You must include at least 1 search keyword*, in order to filter the results by their priority, and whether or not it is completed.
+* The `PRIORITY` must be an integer from 1 to 5, inclusive.
+* `ISTASKDONE` must be either `true` or `false`. If it is `true`, you will only see tasks that have been marked as complete, and if it is `false, you will only see tasks that are not complete, in addition to all other search criteria.
 ****
 
 Examples:

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

@@ -123,13 +123,9 @@ public CommandResult executeUndoableCommand() throws CommandException {
         List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();
         List<ReadOnlyTask> lastShownTaskList = model.getFilteredTaskList();
 
-        if (index.getZeroBased() >= lastShownList.size()) {
-            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
-        }
-
         try {
             if (isTask) {
-                if (index.getZeroBased() >= lastShownList.size()) {
+                if (index.getZeroBased() >= lastShownTaskList.size()) {
                     throw new CommandException(Messages.MESSAGE_INVALID_TASK_DISPLAYED_INDEX);
                 }
                 ReadOnlyTask taskToEdit = lastShownTaskList.get(index.getZeroBased());

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

@@ -20,6 +20,8 @@ public class FindCommand extends Command {
             + "contain any of the specified keywords (case-sensitive) and displays them as a list with index numbers.\n"
             + "Parameters: KEYWORD [MORE_KEYWORDS]...\n"
             + "Example: " + COMMAND_WORD + " task make";
+    public static final String MESSAGE_INVALID_COMPLETE_VALUE = "The task status should either be 'true' or 'false'";
+    public static final String MESSAGE_INVALID_PRIORITY = "The specified priority should be an integer from 1 to 5";
 
     private final NameContainsKeywordsPredicate personPredicate;
     private final TaskContainsKeywordPredicate taskPredicate;

src/main/java/seedu/address/logic/parser/AddCommandParser.java

@@ -51,10 +51,7 @@ public class AddCommandParser implements Parser<AddCommand> {
      */
     public AddCommand parse(String args) throws ParseException {
         ArgumentMultimap argMultimap =
-                ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_REMARK,
-                        PREFIX_REMARK_PRIVATE, PREFIX_TAG, PREFIX_NAME_PRIVATE, PREFIX_PHONE_PRIVATE,
-                        PREFIX_EMAIL_PRIVATE, PREFIX_ADDRESS_PRIVATE, PREFIX_TAG_PRIVATE, PREFIX_DEADLINE,
-                        PREFIX_DESCRIPTION, PREFIX_PRIORITY, PREFIX_TASK);
+                ArgumentTokenizer.tokenize(args, PREFIX_TASK);
         if (arePrefixesPresent(argMultimap, PREFIX_TASK)) {
             ReadOnlyTask taskToAdd = constructTask(args);
             return new AddCommand(taskToAdd);

src/main/java/seedu/address/logic/parser/CliSyntax.java

@@ -24,7 +24,8 @@ public class CliSyntax {
     public static final Prefix PREFIX_TASK = new Prefix("task");
     public static final Prefix PREFIX_DEADLINE = new Prefix("by/");
     public static final Prefix PREFIX_DESCRIPTION = new Prefix("d/");
-    public static final Prefix PREFIX_PRIORITY = new Prefix("pri/");
+    public static final Prefix PREFIX_PRIORITY = new Prefix("p/");
     public static final Prefix PREFIX_TARGET = new Prefix("to/");
     public static final Prefix PREFIX_FROM = new Prefix("from/");
+    public static final Prefix PREFIX_STATE = new Prefix("done/");
 }

src/main/java/seedu/address/logic/parser/FindCommandParser.java

@@ -1,13 +1,16 @@
 package seedu.address.logic.parser;
 
 import static seedu.address.commons.core.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_PRIORITY;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_STATE;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TASK;
 
 import java.util.Arrays;
 
 import seedu.address.logic.commands.FindCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
 import seedu.address.model.person.NameContainsKeywordsPredicate;
+import seedu.address.model.task.Priority;
 import seedu.address.model.task.TaskContainsKeywordPredicate;
 
 /**
@@ -21,7 +24,7 @@ public class FindCommandParser implements Parser<FindCommand> {
      * @throws ParseException if the user input does not conform the expected format
      */
     public FindCommand parse(String args) throws ParseException {
-        ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_TASK);
+        ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_TASK, PREFIX_PRIORITY, PREFIX_STATE);
         String trimmedArgs = args.trim();
 
         if (!argMultimap.getValue(PREFIX_TASK).isPresent()) {
@@ -33,13 +36,60 @@ public FindCommand parse(String args) throws ParseException {
             return new FindCommand(new NameContainsKeywordsPredicate(Arrays.asList(nameKeywords)));
         } else {
             String argsWithNoTaskPrefix = args.replaceFirst(PREFIX_TASK.getPrefix(), "");
-            trimmedArgs = argsWithNoTaskPrefix.trim();
+            argMultimap = ArgumentTokenizer.tokenize(argsWithNoTaskPrefix, PREFIX_PRIORITY, PREFIX_STATE);
+            String keywords = argMultimap.getPreamble();
+            trimmedArgs = keywords.trim();
+            boolean isPriorityFindRequired = argMultimap.getValue(PREFIX_PRIORITY).isPresent();
+            boolean isStateFindRequired = argMultimap.getValue(PREFIX_STATE).isPresent();
+            int minPriority = 0;
+            boolean isComplete = false;
+            if (isPriorityFindRequired) {
+                minPriority = parsePriority(argMultimap.getValue(PREFIX_PRIORITY).get());
+            }
+            if (isStateFindRequired) {
+                isComplete = parseState(argMultimap.getValue(PREFIX_STATE).get());
+            }
             if (trimmedArgs.isEmpty()) {
                 throw new ParseException(
                         String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_TASK_USAGE));
             }
             String[] nameKeywords = trimmedArgs.split("\\s+");
-            return new FindCommand(new TaskContainsKeywordPredicate(Arrays.asList(nameKeywords)));
+
+            return new FindCommand(new TaskContainsKeywordPredicate(Arrays.asList(nameKeywords), isStateFindRequired,
+                    isPriorityFindRequired, isComplete, minPriority));
+        }
+    }
+
+    /**
+     * Parses the given string, and returns an integer corresponding to its value
+     * Guarantees: The specified value is valid as a priority value
+     */
+    public int parsePriority(String args) throws ParseException {
+        if (args == null) {
+            throw new ParseException(Priority.MESSAGE_PRIORITY_CONSTRAINTS);
+        }
+        int priority;
+        try {
+            priority = Integer.parseInt(args.trim());
+        } catch (NumberFormatException nfe) {
+            throw new ParseException(FindCommand.MESSAGE_INVALID_PRIORITY);
+        }
+        if (priority < 1 || priority > 5) {
+            throw new ParseException(FindCommand.MESSAGE_INVALID_PRIORITY);
+        } else {
+            return priority;
+        }
+    }
+
+    /**
+     * Parses the given string, and returns a boolean value corresponding to its value
+     */
+    public boolean parseState(String args) throws ParseException {
+        String trimmed = args.trim();
+        if ("true".equals(trimmed) || "false".equals(trimmed)) {
+            return Boolean.valueOf(trimmed);
+        } else {
+            throw new ParseException(FindCommand.MESSAGE_INVALID_COMPLETE_VALUE);
         }
     }
 }

src/main/java/seedu/address/model/task/Priority.java

@@ -10,36 +10,33 @@ public class Priority {
 
     public static final String MESSAGE_PRIORITY_CONSTRAINTS =
             "Task priorities must be an integer from 1 to 5, inclusive, where 1 represents the lowest priority";
+    public static final String[] PRIORITY_TEXT_STRINGS = {"", "Lowest", "Low", "Medium", "High", "Highest"};
 
-    public static final int PRIORITY_LOWER_BOUND = 1;
+    public static final int PRIORITY_LOWER_BOUND = 0;
     public static final int PRIORITY_UPPER_BOUND = 5;
     public static final String PRIORITY_VALIDATION_REGEX = "[\\d].*";
     public static final String PRIORITY_PLACEHOLDER_VALUE = "";
-    public final String value;
+    public final int value;
 
     /**
      * Validates given priority.
      *
      * @throws IllegalValueException if given priority string is invalid.
      */
     public Priority(String priority) throws IllegalValueException {
-        if (priority == null) {
-            this.value = PRIORITY_PLACEHOLDER_VALUE;
-            return;
-        } else if (priority.equals(PRIORITY_PLACEHOLDER_VALUE)) {
-            this.value = PRIORITY_PLACEHOLDER_VALUE;
+        if (priority == null || priority.equals(PRIORITY_PLACEHOLDER_VALUE)) {
+            this.value = 0;
             return;
         }
         String trimmedPriority = priority.trim();
         try {
-            Integer.parseInt(trimmedPriority);
+            this.value = Integer.parseInt(trimmedPriority);
         } catch (NumberFormatException e) {
             throw new IllegalValueException(MESSAGE_PRIORITY_CONSTRAINTS);
         }
         if (!isValidPriority(trimmedPriority)) {
             throw new IllegalValueException(MESSAGE_PRIORITY_CONSTRAINTS);
         }
-        this.value = trimmedPriority;
     }
 
     /**
@@ -65,19 +62,13 @@ public static boolean isWithinBounds(int test) {
 
     @Override
     public String toString() {
-        return value;
+        return PRIORITY_TEXT_STRINGS[value];
     }
 
     @Override
     public boolean equals(Object other) {
         return other == this // short circuit if same object
                 || (other instanceof Priority // instanceof handles nulls
-                && this.value.equals(((Priority) other).value)); // state check
+                && this.value == ((Priority) other).value); // state check
     }
-
-    @Override
-    public int hashCode() {
-        return value.hashCode();
-    }
-
 }

src/main/java/seedu/address/model/task/ReadOnlyTask.java

@@ -31,6 +31,7 @@ default String getAsText() {
                 .append(getDeadline())
                 .append(" Priority: ")
                 .append(getPriority())
+                .append(" ")
                 .append(getPrintableState());
         return builder.toString();
     }

src/main/java/seedu/address/model/task/TaskContainsKeywordPredicate.java

@@ -11,19 +11,39 @@
  */
 public class TaskContainsKeywordPredicate  implements Predicate<ReadOnlyTask> {
     private final List<String> keywords;
+    private boolean needFilterByState;
+    private boolean needFilterByPriority;
+    private boolean isComplete;
+    private int basePriority;
 
-    public TaskContainsKeywordPredicate(List<String> keywords) {
+    public TaskContainsKeywordPredicate(List<String> keywords, boolean isStateCheckRequired,
+                                        boolean isPriorityCheckRequired, boolean isComplete, int basePriority) {
         this.keywords = keywords;
+        this.needFilterByPriority = isPriorityCheckRequired;
+        this.needFilterByState = isStateCheckRequired;
+        this.isComplete = isComplete;
+        this.basePriority = basePriority;
+    }
 
+    public TaskContainsKeywordPredicate(List<String> keywords) {
+        this.keywords = keywords;
+        this.needFilterByPriority = false;
+        this.needFilterByState = false;
+        this.isComplete = false;
+        this.basePriority = 0;
     }
 
     @Override
     public boolean test(ReadOnlyTask task) {
         for (int i = 0; i < keywords.size(); i++) {
             String keyword = keywords.get(i);
-            if (StringUtil.containsWordIgnoreCase(task.getTaskName().taskName, keyword)
-                    || StringUtil.containsWordIgnoreCase(task.getDescription().value, keyword)) {
-                return true;
+            if (needFilterByState && task.getCompleteState() != isComplete) {
+                return false;
+            } else if (needFilterByPriority && task.getPriority().value < basePriority) {
+                return false;
+            } else {
+                return (StringUtil.containsWordIgnoreCase(task.getTaskName().taskName, keyword)
+                        || StringUtil.containsWordIgnoreCase(task.getDescription().value, keyword));
             }
         }
         return false;
@@ -33,6 +53,10 @@ public boolean test(ReadOnlyTask task) {
     public boolean equals(Object other) {
         return other == this // short circuit if same object
                 || (other instanceof TaskContainsKeywordPredicate // instanceof handles nulls
-                && this.keywords.equals(((TaskContainsKeywordPredicate) other).keywords)); // state check
+                && this.keywords.equals(((TaskContainsKeywordPredicate) other).keywords)
+                && this.needFilterByPriority == ((TaskContainsKeywordPredicate) other).needFilterByPriority
+                && this.needFilterByState == ((TaskContainsKeywordPredicate) other).needFilterByState
+                && this.isComplete == ((TaskContainsKeywordPredicate) other).isComplete
+                && this.basePriority == ((TaskContainsKeywordPredicate) other).basePriority); // state check
     }
 }

src/main/java/seedu/address/model/task/UniqueTaskList.java

@@ -123,7 +123,7 @@ public void sortBy(String field, String order) {
         Comparator<Task> priorityComparator = new Comparator<Task>() {
             @Override
             public int compare(Task o1, Task o2) {
-                return o1.getPriority().value.compareTo(o2.getPriority().value);
+                return Integer.compare(o1.getPriority().value, o2.getPriority().value);
             }
         };
 

src/main/java/seedu/address/storage/XmlAdaptedTask.java

@@ -41,7 +41,7 @@ public XmlAdaptedTask(ReadOnlyTask source) {
         name = source.getTaskName().taskName;
         description = source.getDescription().value;
         deadline = source.getDeadline().value;
-        priority = source.getPriority().value;
+        priority = Integer.toString(source.getPriority().value);
         state = String.valueOf(source.getCompleteState());
     }
 

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

@@ -269,7 +269,7 @@ public void parseCommandFind() throws Exception {
         command = (FindCommand) parser.parseCommand(
                 FindCommand.COMMAND_WORD + " " + PREFIX_TASK +  " "
                         + keywords.stream().collect(Collectors.joining(" ")));
-        assertEquals(new FindCommand(new TaskContainsKeywordPredicate(keywords)), command);
+        assertEquals(new FindCommand(new TaskContainsKeywordPredicate(keywords, false, false, false, 0)), command);
     }
 
     @Test
@@ -282,7 +282,7 @@ public void parseCommandAliasFind() throws Exception {
         command = (FindCommand) parser.parseCommand(
                 FindCommand.COMMAND_ALIAS + " " + PREFIX_TASK +  " "
                         + keywords.stream().collect(Collectors.joining(" ")));
-        assertEquals(new FindCommand(new TaskContainsKeywordPredicate(keywords)), command);
+        assertEquals(new FindCommand(new TaskContainsKeywordPredicate(keywords, false, false, false, 0)), command);
     }
     //@@author wangyiming1019
     @Test

src/test/java/seedu/address/model/UniqueTaskListTest.java

@@ -66,7 +66,7 @@ public void sortTasks_byPriority_bothOrders() {
         Comparator<ReadOnlyTask> deadlineComparator = new Comparator<ReadOnlyTask>() {
             @Override
             public int compare(ReadOnlyTask o1, ReadOnlyTask o2) {
-                return o1.getPriority().value.compareTo(o2.getPriority().value);
+                return Integer.compare(o1.getPriority().value, o2.getPriority().value);
             }
         };
         Collections.sort(taskList, deadlineComparator);

src/test/java/seedu/address/model/task/PriorityTest.java

@@ -14,7 +14,7 @@ public void isValidPriority() {
         assertFalse(Priority.isValidPriority(" ")); // spaces only
         assertFalse(Priority.isValidPriority("invalid")); // invalid priority
         assertFalse(Priority.isValidPriority("777")); // priority out of range
-        assertFalse(Priority.isValidPriority("0")); // priority out of range
+        assertFalse(Priority.isValidPriority("-1")); // priority out of range
 
         // valid names
         assertTrue(Priority.isValidPriority("")); // empty string