Merge pull request #125 from sopa301/branch-enhancement-findMore

Branch enhancement find more

docs/DeveloperGuide.md

@@ -159,6 +159,43 @@ Classes used by multiple components are in the `seedu.addressbook.commons` packa
 
 This section describes some noteworthy details on how certain features are implemented.
 
+### Expanded Find feature
+
+The enhanced find mechanism is facilitated by the `CombinedPredicate` and utilises the existing FindCommand structure. 
+It extends `Predicate<Person>` and is composed of up to one of a `NameContainsKeywordsPredicate`, `FinancialPlanContainsKeywordsPredicate`
+and a `TagContainsKeywordsPredicate` each. Here's a partial class diagram of the `CombinedPredicate`.
+![CombinedPredicate](images/CombinedPredicate.png)
+
+The `NameContainsKeywordsPredicate`, `FinancialPlanContainsKeywordsPredicate` and
+`TagContainsKeywordsPredicate` check a Person if the respective field contains
+any of the keywords supplied to the predicate. Note that only the `NameContainsKeywordsPredicate` 
+checks for whole words, because it is rare to search for people by substrings, while `FinancialPlanContainsKeywordsPredicate`
+and `TagContainsKeywordsPredicate` allow matching for substrings because there are certain cases where it is logical to search for
+substrings e.g. `Plan A` and `Plan A Premium` are related, so they can show up in the same search.
+
+The Find command format also changes to resemble a format more similar to the `add` and `edit` commands, to allow for
+searching for keywords in multiple fields at the same time.
+
+#### Design Considerations:
+
+**Aspect: How to implement find for multiple fields**
+* **Alternative 1 (current choice):** Use one unified command and format.
+    * Pros: Easy to implement (argument multimap is available), allows for more flexible usage.
+    * Cons: May get cluttered when there are many terms.
+
+* **Alternative 2:** Take an argument to decide which field to find by.
+    * Pros: More user-friendly and natural since there is no need to use prefixes.
+    * Cons: Less flexible, slightly more difficult to implement.
+
+**Aspect: How to implement `CombinedPredicate`**
+* **Alternative 1 (current choice):** Compose it with the 3 component predicates.
+    * Pros: Easier to modify and test.
+    * Cons: Less flexible when trying to combine multiple predicates (that may be of the same type).
+
+* **Alternative 2:** Use a `Predicate<Person>` and use the `or()` method to chain predicates.
+    * Pros: More flexible in usage.
+    * Cons: More difficult to modify and test.
+
 ### \[Proposed\] Undo/redo feature
 
 #### Proposed Implementation

docs/UserGuide.md

@@ -150,20 +150,20 @@ Next-of-Kin Phone: 82020202
 
 ### Locating persons by name: `find`
 
-Finds persons whose names contain any of the given keywords.
+Finds persons whose names, tags or financial plans contain any of the specified keywords.
 
-Format: `find KEYWORD [MORE_KEYWORDS]`
+Format: `find [n/NAME]…​ [fp/FINANCIAL_PLAN]…​ [t/TAG]…​`
 
+* At least one of the optional fields must be provided.
 * The search is case-insensitive. e.g `hans` will match `Hans`
-* The order of the keywords does not matter. e.g. `Hans Bo` will match `Bo Hans`
-* Only the name is searched.
-* Only full words will be matched e.g. `Han` will not match `Hans`
+* For names, only full words will be matched e.g. `Han` will not match `Hans`
+* For financial plans and tags, any substring will be matched e.g. `Senior` will match `SuperSenior`
 * Persons matching at least one keyword will be returned (i.e. `OR` search).
-  e.g. `Hans Bo` will return `Hans Gruber`, `Bo Yang`
+  e.g. `n/Hans n/Bo` will return `Hans Gruber`, `Bo Yang`
 
 Examples:
-* `find John` returns `john` and `John Doe`
-* `find alex david` returns `Alex Yeoh`, `David Li`<br>
+* `find n/John` returns `john` and `John Doe`
+* `find n/alex n/david` returns `Alex Yeoh`, `David Li`<br>
   ![result for 'find alex david'](images/findAlexDavidResult.png)
 
 ### Gathering clients' emails by financial plan: `gather`
@@ -269,7 +269,7 @@ _Details coming soon ..._
 | **Clear**  | `clear`                                                                                                                                                                                                                 |
 | **Delete** | `delete INDEX`<br> e.g., `delete 3`                                                                                                                                                                                     |
 | **Edit**   | `edit ENTRY_INDEX [n/NAME] [p/PHONE_NUMBER] [e/EMAIL] [a/ADDRESS] [nk/NEXT_KIN] [nkp/NEXT_KIN_PHONE] [t/TAG]…​`<br> e.g.,`edit 1 n/john doe a/23 woodlands ave 123`                                                     |
-| **Find**   | `find KEYWORD [MORE_KEYWORDS]`<br> e.g., `find James Jake`                                                                                                                                                              |
+| **Find**   | `find [n/NAME]…​ [fp/FINANCIAL_PLAN]…​ [t/TAG]…​`<br> e.g., `find n/James n/Jake`                                                                                                                                       |
 | **Gather** | `gather FINANCIAL_PLAN_NAME` <br> e.g., `gather Basic Insurance Plan`                                                                                                                                                   |
 | **List**   | `list`                                                                                                                                                                                                                  |
 | **Help**   | `help`                                                                                                                                                                                                                  |

docs/diagrams/CombinedPredicateDiagram.puml

@@ -0,0 +1,15 @@
+@startuml
+!include style.puml
+skinparam arrowThickness 1.1
+skinparam arrowColor LOGIC_COLOR_T4
+skinparam classBackgroundColor LOGIC_COLOR
+
+CombinedPredicate .up.|> "Predicate<Person>"
+CombinedPredicate --> "0...1" NameContainsKeywordsPredicate
+CombinedPredicate --> "0...1" FinancialPlanContainsKeywordsPredicate
+CombinedPredicate --> "0...1" TagContainsKeywordsPredicate
+
+FindCommandParser ..> CombinedPredicate : creates >
+FindCommandParser ..> FindCommand : creates >
+FindCommand-->CombinedPredicate
+@enduml

src/main/java/seedu/address/commons/util/StringUtil.java

@@ -6,6 +6,9 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * Helper functions for handling strings.
@@ -38,6 +41,47 @@ public static boolean containsWordIgnoreCase(String sentence, String word) {
                 .anyMatch(preppedWord::equalsIgnoreCase);
     }
 
+    /**
+     * Returns true if the {@code sentence} contains the {@code words}.
+     *   Ignores case, but a full phrase match is required.
+     *   <br>examples:<pre>
+     *       containsWordsIgnoreCase("ABc def", "abc def") == true
+     *       containsWordsIgnoreCase("ABc def", "DEF") == true
+     *       containsWordsIgnoreCase("ABc def", "ABc de") == false //not a full word match
+     *       </pre>
+     * @param sentence cannot be null
+     * @param words cannot be null, cannot be empty
+     */
+    public static boolean containsWordsIgnoreCase(String sentence, String words) {
+        requireNonNull(sentence);
+        requireNonNull(words);
+
+        String trimmedWords = words.trim();
+        checkArgument(!trimmedWords.isEmpty(), "Word parameter cannot be empty");
+        List<String> preppedWords = prepareWords(trimmedWords);
+
+        String trimmedSentence = sentence.trim();
+        if (trimmedSentence.isEmpty()) {
+            return false;
+        }
+        List<String> preppedSentence = prepareWords(sentence);
+
+        return Collections.indexOfSubList(preppedSentence, preppedWords) != -1;
+    }
+
+    /**
+     * Prepares the given words to be compared in the containsWordsIgnoreCase() method.
+     *
+     * @param words cannot be null
+     * @return the prepared list of strings
+     */
+    private static List<String> prepareWords(String words) {
+        requireNonNull(words);
+        return Arrays.stream(words.split("\\s+"))
+                .map(word -> word.toLowerCase())
+                .collect(Collectors.toList());
+    }
+
     /**
      * Returns a detailed message of the t, including the stack trace.
      */

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

@@ -1,28 +1,39 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_FINANCIAL_PLAN;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
+
+import java.util.function.Predicate;
 
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.Messages;
 import seedu.address.model.Model;
-import seedu.address.model.person.NameContainsKeywordsPredicate;
+import seedu.address.model.person.Person;
 
 /**
- * Finds and lists all persons in address book whose name contains any of the argument keywords.
- * Keyword matching is case insensitive.
+ * Finds and lists all persons in address book whose name, tags or financial plans contains any of the argument
+ * keywords. Keyword matching is case-insensitive.
  */
 public class FindCommand extends Command {
 
     public static final String COMMAND_WORD = "find";
 
-    public static final String MESSAGE_USAGE = COMMAND_WORD + ": Finds all persons whose names contain any of "
-            + "the specified keywords (case-insensitive) and displays them as a list with index numbers.\n"
-            + "Parameters: KEYWORD [MORE_KEYWORDS]...\n"
-            + "Example: " + COMMAND_WORD + " alice bob charlie";
+    public static final String MESSAGE_USAGE = COMMAND_WORD + ": Finds all persons whose names, tags or financial"
+            + " plans contain any of "
+            + "the specified keywords (case-insensitive) and displays them as a list with index numbers. "
+            + "At least one argument must be given.\n"
+            + "Parameters: "
+            + "[" + PREFIX_NAME + "NAME]... "
+            + "[" + PREFIX_FINANCIAL_PLAN + "FINANCIAL_PLAN]... "
+            + "[" + PREFIX_TAG + "TAG]...\n"
+            + "Example: " + COMMAND_WORD + " " + PREFIX_FINANCIAL_PLAN + "Financial Plan A "
+            + PREFIX_TAG + "TagA";
 
-    private final NameContainsKeywordsPredicate predicate;
+    private final Predicate<Person> predicate;
 
-    public FindCommand(NameContainsKeywordsPredicate predicate) {
+    public FindCommand(Predicate<Person> predicate) {
         this.predicate = predicate;
     }
 

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

@@ -1,12 +1,21 @@
 package seedu.address.logic.parser;
 
 import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_FINANCIAL_PLAN;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
+import static seedu.address.logic.parser.ParserUtil.validateFinancialPlans;
+import static seedu.address.logic.parser.ParserUtil.validateNames;
+import static seedu.address.logic.parser.ParserUtil.validateTags;
 
-import java.util.Arrays;
+import java.util.List;
 
 import seedu.address.logic.commands.FindCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.person.NameContainsKeywordsPredicate;
+import seedu.address.model.person.predicates.CombinedPredicate;
+import seedu.address.model.person.predicates.FinancialPlanContainsKeywordsPredicate;
+import seedu.address.model.person.predicates.NameContainsKeywordsPredicate;
+import seedu.address.model.person.predicates.TagContainsKeywordsPredicate;
 
 /**
  * Parses input arguments and creates a new FindCommand object
@@ -20,14 +29,32 @@ public class FindCommandParser implements Parser<FindCommand> {
      */
     public FindCommand parse(String args) throws ParseException {
         String trimmedArgs = args.trim();
-        if (trimmedArgs.isEmpty()) {
+        ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_NAME,
+                PREFIX_FINANCIAL_PLAN, PREFIX_TAG);
+        if (trimmedArgs.isEmpty() || !argMultimap.getPreamble().isEmpty()) {
             throw new ParseException(
                     String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE));
         }
 
-        String[] nameKeywords = trimmedArgs.split("\\s+");
+        List<String> nameKeywords = argMultimap.getAllValues(PREFIX_NAME);
+        nameKeywords.replaceAll(name -> name.trim());
+        validateNames(nameKeywords);
 
-        return new FindCommand(new NameContainsKeywordsPredicate(Arrays.asList(nameKeywords)));
+        List<String> tagKeywords = argMultimap.getAllValues(PREFIX_TAG);
+        tagKeywords.replaceAll(tag -> tag.trim());
+        validateTags(tagKeywords);
+
+        List<String> financialPlanKeywords = argMultimap.getAllValues(PREFIX_FINANCIAL_PLAN);
+        financialPlanKeywords.replaceAll(financialPlan -> financialPlan.trim());
+        validateFinancialPlans(financialPlanKeywords);
+
+        CombinedPredicate combinedPredicate = new CombinedPredicate(
+                new FinancialPlanContainsKeywordsPredicate(financialPlanKeywords),
+                new NameContainsKeywordsPredicate(nameKeywords),
+                new TagContainsKeywordsPredicate(tagKeywords)
+        );
+
+        return new FindCommand(combinedPredicate);
     }
 
 }

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

@@ -3,13 +3,13 @@
 import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_FINANCIAL_PLAN;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
+import static seedu.address.logic.parser.ParserUtil.validateFinancialPlan;
+import static seedu.address.logic.parser.ParserUtil.validateTag;
 
 import seedu.address.logic.commands.GatherCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.financialplan.FinancialPlan;
 import seedu.address.model.person.gatheremail.GatherEmailByFinancialPlan;
 import seedu.address.model.person.gatheremail.GatherEmailByTag;
-import seedu.address.model.tag.Tag;
 
 /**
  * Parses input arguments and creates a new GatherCommand object
@@ -45,19 +45,4 @@ public GatherCommand parse(String args) throws ParseException {
         throw new ParseException(
                 String.format(MESSAGE_INVALID_COMMAND_FORMAT, GatherCommand.MESSAGE_USAGE));
     }
-
-    private void validateFinancialPlan(String input) throws ParseException {
-        if (input.isEmpty() || !FinancialPlan.isValidFinancialPlanName(input)) {
-            throw new ParseException(
-                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, FinancialPlan.MESSAGE_CONSTRAINTS));
-        }
-    }
-
-    private void validateTag(String input) throws ParseException {
-        if (input.isEmpty() || !Tag.isValidTagName(input)) {
-            throw new ParseException(
-                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, Tag.MESSAGE_CONSTRAINTS));
-        }
-    }
-
 }

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

@@ -1,11 +1,13 @@
 package seedu.address.logic.parser;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
 
 import java.time.LocalDateTime;
 import java.time.format.DateTimeParseException;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 import seedu.address.commons.core.index.Index;
@@ -22,7 +24,7 @@
 import seedu.address.model.tag.Tag;
 
 /**
- * Contains utility methods used for parsing strings in the various *Parser classes.
+ * Contains utility methods used for parsing and validating strings in the various *Parser classes.
  */
 public class ParserUtil {
     public static final String MESSAGE_INVALID_INDEX = "Index is not a non-zero unsigned integer.";
@@ -215,4 +217,74 @@ public static Set<Tag> parseTags(Collection<String> tags) throws ParseException
         }
         return tagSet;
     }
+    /**
+     * Validates if a {@code String name} is a valid {@code Name}.
+     *
+     * @param input String to validate.
+     * @throws ParseException if the given string is invalid.
+     */
+    public static void validateName(String input) throws ParseException {
+        if (input.isEmpty() || !Name.isValidName(input)) {
+            throw new ParseException(
+                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, Name.MESSAGE_CONSTRAINTS));
+        }
+    }
+    /**
+     * Validates if a list of {@code String names} are valid for {@code Name} objects.
+     *
+     * @param inputs List of names to validate.
+     * @throws ParseException if any of the given names are invalid.
+     */
+    public static void validateNames(List<String> inputs) throws ParseException {
+        for (String name : inputs) {
+            validateName(name);
+        }
+    }
+    /**
+     * Validates if a {@code String financial plan} is a valid name for a {@code FinancialPlan}.
+     *
+     * @param input String to validate.
+     * @throws ParseException if the given string is invalid.
+     */
+    public static void validateFinancialPlan(String input) throws ParseException {
+        if (input.isEmpty() || !FinancialPlan.isValidFinancialPlanName(input)) {
+            throw new ParseException(
+                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, FinancialPlan.MESSAGE_CONSTRAINTS));
+        }
+    }
+    /**
+     * Validates if a list of {@code String financial plans} are valid as {@code FinancialPlan} names.
+     *
+     * @param inputs List of financial plan names to validate.
+     * @throws ParseException if any of the given names are invalid.
+     */
+    public static void validateFinancialPlans(List<String> inputs) throws ParseException {
+        for (String financialPlan : inputs) {
+            validateFinancialPlan(financialPlan);
+        }
+    }
+
+    /**
+     * Validates if a {@code String tag} is a valid name for a {@code Tag}.
+     *
+     * @param input String to validate.
+     * @throws ParseException if the given string is invalid.
+     */
+    public static void validateTag(String input) throws ParseException {
+        if (input.isEmpty() || !Tag.isValidTagName(input)) {
+            throw new ParseException(
+                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, Tag.MESSAGE_CONSTRAINTS));
+        }
+    }
+    /**
+     * Validates if a list of {@code String tags} are valid as {@code Tag} names.
+     *
+     * @param inputs List of tag names to validate.
+     * @throws ParseException if any of the given names are invalid.
+     */
+    public static void validateTags(List<String> inputs) throws ParseException {
+        for (String tag : inputs) {
+            validateTag(tag);
+        }
+    }
 }