From 56bfd33ec2e4e25a30dfd7cf4671e97421142474 Mon Sep 17 00:00:00 2001 From: rizkidelta Date: Fri, 29 Mar 2024 14:10:31 +0800 Subject: [PATCH 01/13] Implement Fuzzy Search in Find Command Added fuzzy search functionality to the existing find command. This enhancement allows for more forgiving search queries, improving user experience by returning relevant results even when the search term isn't an exact match. --- .../PersonMatchesKeywordsPredicate.java | 61 +++++++++++++++---- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java index ec037a5a708..883790d2f88 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java @@ -3,7 +3,6 @@ import java.util.List; import java.util.function.Predicate; -import seedu.address.commons.util.StringUtil; import seedu.address.commons.util.ToStringBuilder; /** @@ -20,17 +19,55 @@ public PersonMatchesKeywordsPredicate(List keywords) { @Override public boolean test(Person person) { return keywords.stream() - .anyMatch(keyword -> { - boolean nameContainsKeyword = - StringUtil.containsWordIgnoreCase(person.getName().toString(), keyword); - boolean tagsContainKeyword = person.getTags() - .stream() - .anyMatch(tag -> StringUtil.containsWordIgnoreCase(tag.get(), keyword)); - boolean assetsContainKeyword = person.getAssets() - .stream() - .anyMatch(asset -> StringUtil.containsWordIgnoreCase(asset.get(), keyword)); - return nameContainsKeyword || tagsContainKeyword || assetsContainKeyword; - }); + .anyMatch(keyword -> fuzzyMatch(person.getName().toString(), keyword) || + person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) || + person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); + } + + + private int calculateLevenshteinDistance(String a, String b) { + a = a.toLowerCase(); + b = b.toLowerCase(); + int[] costs = new int[b.length() + 1]; + + for (int j = 0; j < costs.length; j++) { + costs[j] = j; + } + + for (int i = 1; i <= a.length(); i++) { + costs[0] = i; + int nw = i - 1; + + for (int j = 1; j <= b.length(); j++) { + int cj = Math.min(1 + Math.min(costs[j], costs[j - 1]), a.charAt(i - 1) == b.charAt(j - 1) ? nw : nw + 1); + nw = costs[j]; + costs[j] = cj; + } + } + + return costs[b.length()]; + } + + private boolean fuzzyMatch(String text, String keyword) { + text = text.toLowerCase(); + keyword = keyword.toLowerCase(); + + // Split the text into words + String[] words = text.split("\\s+"); + + for (String word : words) { + int levenshteinDistance = calculateLevenshteinDistance(word, keyword); + + // Calculate the similarity based on the length of the longer string (word or keyword) + int longerLength = Math.max(word.length(), keyword.length()); + double similarity = 1.0 - (double) levenshteinDistance / longerLength; + + if (similarity >= 0.7) { + return true; + } + } + + return false; } @Override From c3027e9bb877100905fbb8a132a61516af8d1bba Mon Sep 17 00:00:00 2001 From: rizkidelta Date: Sun, 31 Mar 2024 13:35:58 +0800 Subject: [PATCH 02/13] Update Fuzzy search Implemented Fuzzy search so users can easily search within the system even if the user mistype a few words, so that. --- .../model/person/PersonMatchesKeywordsPredicate.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java index 883790d2f88..7ad6bffc25f 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java @@ -19,8 +19,10 @@ public PersonMatchesKeywordsPredicate(List keywords) { @Override public boolean test(Person person) { return keywords.stream() - .anyMatch(keyword -> fuzzyMatch(person.getName().toString(), keyword) || - person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) || + .anyMatch(keyword -> fuzzyMatch(person.getName().toString(), keyword) + || + person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) + || person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); } @@ -39,7 +41,8 @@ private int calculateLevenshteinDistance(String a, String b) { int nw = i - 1; for (int j = 1; j <= b.length(); j++) { - int cj = Math.min(1 + Math.min(costs[j], costs[j - 1]), a.charAt(i - 1) == b.charAt(j - 1) ? nw : nw + 1); + int cj = Math.min(1 + Math.min(costs[j], costs[j - 1]), + a.charAt(i - 1) == b.charAt(j - 1) ? nw : nw + 1); nw = costs[j]; costs[j] = cj; } From 3a06daebe2466a4df42c94f2aab3b78c0e5c4992 Mon Sep 17 00:00:00 2001 From: rizkidelta Date: Wed, 3 Apr 2024 17:07:51 +0800 Subject: [PATCH 03/13] Update Fuzzy search Implemented Fuzzy search so users can easily search within the system even if the user mistype a few words, so that and added tests to test fuzzy functionality. --- .../PersonMatchesKeywordsPredicate.java | 6 ++--- .../logic/commands/FindCommandTest.java | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java index 7ad6bffc25f..591b7e0164a 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java @@ -20,10 +20,8 @@ public PersonMatchesKeywordsPredicate(List keywords) { public boolean test(Person person) { return keywords.stream() .anyMatch(keyword -> fuzzyMatch(person.getName().toString(), keyword) - || - person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) - || - person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); + || person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) + || person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); } diff --git a/src/test/java/seedu/address/logic/commands/FindCommandTest.java b/src/test/java/seedu/address/logic/commands/FindCommandTest.java index 7682872f922..db906776bc2 100644 --- a/src/test/java/seedu/address/logic/commands/FindCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/FindCommandTest.java @@ -95,6 +95,28 @@ public void of_validArgs_returnsFindCommand() { assertParseSuccess(FindCommand::of, " \n Alice \n \t Bob \t", expectedFindCommand); } + @Test + public void execute_fuzzyMatching_partialMatchPersonFound() { + String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 1); + // Assuming "Carl Kurz" is in the address book and "Karl" is a fuzzy match for "Carl" + PersonMatchesKeywordsPredicate predicate = preparePredicate("Karl"); + FindCommand command = new FindCommand(predicate); + expectedModel.updateFilteredPersonList(predicate); + assertCommandSuccess(command, model, expectedMessage, expectedModel); + assertEquals(List.of(CARL), model.getFilteredPersonList()); + } + + @Test + public void execute_fuzzyMatching_misspelledKeywordPersonFound() { + String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 1); + // Assuming "Elle Meyer" is in the address book and "Elee" is a fuzzy match for "Elle" + PersonMatchesKeywordsPredicate predicate = preparePredicate("Elee"); + FindCommand command = new FindCommand(predicate); + expectedModel.updateFilteredPersonList(predicate); + assertCommandSuccess(command, model, expectedMessage, expectedModel); + assertEquals(List.of(ELLE), model.getFilteredPersonList()); + } + @Test public void toStringMethod() { PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(List.of("keyword")); From 57ba0779d9e435d9c9dacb150b512f9fe1c976bc Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 18:57:08 +0800 Subject: [PATCH 04/13] Add failing tests --- .../model/person/PersonMatchesKeywordsPredicate.java | 1 - .../person/PersonMatchesKeywordsPredicateTest.java | 12 ++++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java index 591b7e0164a..c1545bb438d 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java @@ -24,7 +24,6 @@ public boolean test(Person person) { || person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); } - private int calculateLevenshteinDistance(String a, String b) { a = a.toLowerCase(); b = b.toLowerCase(); diff --git a/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java b/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java index f28e057285f..091f3665be6 100644 --- a/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java +++ b/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java @@ -60,6 +60,10 @@ public void test_nameContainsKeywords_returnsTrue() { // Mixed-case keywords predicate = new PersonMatchesKeywordsPredicate(List.of("aLIce", "bOB")); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); + + // Short query, long name + predicate = new PersonMatchesKeywordsPredicate(List.of("y")); + assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh The Fifth").build())); } @Test @@ -92,6 +96,10 @@ public void test_tagContainsKeywords_returnsTrue() { // Mixed-case keywords predicate = new PersonMatchesKeywordsPredicate(List.of("fRieNdS", "cOllEaGuEs")); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); + + // Short query, long tag + predicate = new PersonMatchesKeywordsPredicate(List.of("a")); + assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); } @Test @@ -124,6 +132,10 @@ public void test_assetContainsKeywords_returnsTrue() { // Mixed-case keywords predicate = new PersonMatchesKeywordsPredicate(List.of("hAmMeR", "sCrEwDriVer")); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); + + // Short query, long asset + predicate = new PersonMatchesKeywordsPredicate(List.of("a")); + assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); } @Test From 3e5e1e11f732b7b5641e391aadf7bd212e69505e Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 19:58:54 +0800 Subject: [PATCH 05/13] Remove fuzzy search and add substring search --- build.gradle | 2 + .../address/logic/commands/FindCommand.java | 8 +- .../PersonMatchesKeywordsPredicate.java | 93 ------------------- .../person/PersonMatchesSearchPredicate.java | 62 +++++++++++++ .../logic/commands/CommandTestUtil.java | 4 +- .../logic/commands/FindCommandTest.java | 44 +++------ .../logic/util/AddressBookParserTest.java | 4 +- .../seedu/address/model/ModelManagerTest.java | 4 +- ... => PersonMatchesSearchPredicateTest.java} | 72 +++++++------- 9 files changed, 123 insertions(+), 170 deletions(-) delete mode 100644 src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java create mode 100644 src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java rename src/test/java/seedu/address/model/person/{PersonMatchesKeywordsPredicateTest.java => PersonMatchesSearchPredicateTest.java} (63%) diff --git a/build.gradle b/build.gradle index a378812b244..3cbf3339c31 100644 --- a/build.gradle +++ b/build.gradle @@ -60,6 +60,8 @@ dependencies { implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.7.0' implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.7.4' +// implementation group: 'com.productiveedge', name: 'fuzzy-search', version: '1.0' + testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: jUnitVersion testRuntimeOnly group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: jUnitVersion diff --git a/src/main/java/seedu/address/logic/commands/FindCommand.java b/src/main/java/seedu/address/logic/commands/FindCommand.java index 7fa911aa07f..9c5ff03bb7f 100644 --- a/src/main/java/seedu/address/logic/commands/FindCommand.java +++ b/src/main/java/seedu/address/logic/commands/FindCommand.java @@ -8,7 +8,7 @@ import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.model.Model; -import seedu.address.model.person.PersonMatchesKeywordsPredicate; +import seedu.address.model.person.PersonMatchesSearchPredicate; /** * Finds and lists all persons in address book whose name contains any of the argument keywords. @@ -24,9 +24,9 @@ public class FindCommand extends Command { + "Parameters: KEYWORD [MORE_KEYWORDS]...\n" + "Example: " + COMMAND_WORD + " alice bob charlie"; - private final PersonMatchesKeywordsPredicate predicate; + private final PersonMatchesSearchPredicate predicate; - public FindCommand(PersonMatchesKeywordsPredicate predicate) { + public FindCommand(PersonMatchesSearchPredicate predicate) { this.predicate = predicate; } @@ -51,7 +51,7 @@ public static FindCommand of(String args) throws IllegalArgumentException { String[] nameKeywords = trimmedArgs.split("\\s+"); - return new FindCommand(new PersonMatchesKeywordsPredicate(Arrays.asList(nameKeywords))); + return new FindCommand(new PersonMatchesSearchPredicate(Arrays.asList(nameKeywords))); } @Override diff --git a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java deleted file mode 100644 index c1545bb438d..00000000000 --- a/src/main/java/seedu/address/model/person/PersonMatchesKeywordsPredicate.java +++ /dev/null @@ -1,93 +0,0 @@ -package seedu.address.model.person; - -import java.util.List; -import java.util.function.Predicate; - -import seedu.address.commons.util.ToStringBuilder; - -/** - * Tests that a {@code Person}'s {@code Name}, {@code Tags} or {@code Assets} matches any of the keywords given. - */ -public class PersonMatchesKeywordsPredicate implements Predicate { - - private final List keywords; - - public PersonMatchesKeywordsPredicate(List keywords) { - this.keywords = keywords; - } - - @Override - public boolean test(Person person) { - return keywords.stream() - .anyMatch(keyword -> fuzzyMatch(person.getName().toString(), keyword) - || person.getTags().stream().anyMatch(tag -> fuzzyMatch(tag.get(), keyword)) - || person.getAssets().stream().anyMatch(asset -> fuzzyMatch(asset.get(), keyword))); - } - - private int calculateLevenshteinDistance(String a, String b) { - a = a.toLowerCase(); - b = b.toLowerCase(); - int[] costs = new int[b.length() + 1]; - - for (int j = 0; j < costs.length; j++) { - costs[j] = j; - } - - for (int i = 1; i <= a.length(); i++) { - costs[0] = i; - int nw = i - 1; - - for (int j = 1; j <= b.length(); j++) { - int cj = Math.min(1 + Math.min(costs[j], costs[j - 1]), - a.charAt(i - 1) == b.charAt(j - 1) ? nw : nw + 1); - nw = costs[j]; - costs[j] = cj; - } - } - - return costs[b.length()]; - } - - private boolean fuzzyMatch(String text, String keyword) { - text = text.toLowerCase(); - keyword = keyword.toLowerCase(); - - // Split the text into words - String[] words = text.split("\\s+"); - - for (String word : words) { - int levenshteinDistance = calculateLevenshteinDistance(word, keyword); - - // Calculate the similarity based on the length of the longer string (word or keyword) - int longerLength = Math.max(word.length(), keyword.length()); - double similarity = 1.0 - (double) levenshteinDistance / longerLength; - - if (similarity >= 0.7) { - return true; - } - } - - return false; - } - - @Override - public boolean equals(Object other) { - if (other == this) { - return true; - } - - // instanceof handles nulls - if (!(other instanceof PersonMatchesKeywordsPredicate)) { - return false; - } - - PersonMatchesKeywordsPredicate otherPersonMatchesKeywordsPredicate = (PersonMatchesKeywordsPredicate) other; - return keywords.equals(otherPersonMatchesKeywordsPredicate.keywords); - } - - @Override - public String toString() { - return new ToStringBuilder(this).add("keywords", keywords).toString(); - } - -} diff --git a/src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java new file mode 100644 index 00000000000..3ab86d823e3 --- /dev/null +++ b/src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java @@ -0,0 +1,62 @@ +package seedu.address.model.person; + +import java.util.List; +import java.util.function.Predicate; + +import seedu.address.commons.util.ToStringBuilder; + +/** + * Tests that a {@code Person}'s {@code Name}, {@code Tags} or {@code Assets} matches the search query. + * The algorithm removes all whitespaces and checks if the query is a substring of a field. + * The substring matching is case-insensitive. + */ +public class PersonMatchesSearchPredicate implements Predicate { + + private final List keywords; + + public PersonMatchesSearchPredicate(List keywords) { + this.keywords = keywords; + } + + //@@author rizkidelta + @Override + public boolean test(Person person) { + return keywords.stream() + .anyMatch(keyword -> substringMatch(person.getName().toString(), keyword) + || person.getTags().stream().anyMatch(tag -> substringMatch(tag.get(), keyword)) + || person.getAssets().stream().anyMatch(asset -> substringMatch(asset.get(), keyword))); + } + + //@@author rizkidelta + private boolean substringMatch(String text, String query) { + final String whitespaceRegex = "\\s+"; + final String emptyString = ""; + + // remove all whitespaces from strings + text = text.toLowerCase().replaceAll(whitespaceRegex, emptyString); + query = query.toLowerCase().replaceAll(whitespaceRegex, emptyString); + + return text.contains(query); + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + + // instanceof handles nulls + if (!(other instanceof PersonMatchesSearchPredicate)) { + return false; + } + + PersonMatchesSearchPredicate otherPersonMatchesSearchPredicate = (PersonMatchesSearchPredicate) other; + return keywords.equals(otherPersonMatchesSearchPredicate.keywords); + } + + @Override + public String toString() { + return new ToStringBuilder(this).add("keywords", keywords).toString(); + } + +} diff --git a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java index 80642822093..61965d43cb5 100644 --- a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java +++ b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java @@ -19,7 +19,7 @@ import seedu.address.model.AddressBook; import seedu.address.model.Model; import seedu.address.model.person.Person; -import seedu.address.model.person.PersonMatchesKeywordsPredicate; +import seedu.address.model.person.PersonMatchesSearchPredicate; import seedu.address.testutil.EditPersonDescriptorBuilder; /** @@ -118,7 +118,7 @@ public static void showPersonAtIndex(Model model, Index targetIndex) { Person person = model.getFilteredPersonList().get(targetIndex.getZeroBased()); final String[] splitName = person.getName().toString().split("\\s+"); - model.updateFilteredPersonList(new PersonMatchesKeywordsPredicate(Arrays.asList(splitName[0]))); + model.updateFilteredPersonList(new PersonMatchesSearchPredicate(Arrays.asList(splitName[0]))); assertEquals(1, model.getFilteredPersonList().size()); } diff --git a/src/test/java/seedu/address/logic/commands/FindCommandTest.java b/src/test/java/seedu/address/logic/commands/FindCommandTest.java index db906776bc2..0129185d6e5 100644 --- a/src/test/java/seedu/address/logic/commands/FindCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/FindCommandTest.java @@ -21,7 +21,7 @@ import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; -import seedu.address.model.person.PersonMatchesKeywordsPredicate; +import seedu.address.model.person.PersonMatchesSearchPredicate; /** * Contains integration tests (interaction with the Model) for {@code FindCommand}. @@ -33,10 +33,10 @@ public class FindCommandTest { @Test public void equals() { - PersonMatchesKeywordsPredicate firstPredicate = - new PersonMatchesKeywordsPredicate(Collections.singletonList("first")); - PersonMatchesKeywordsPredicate secondPredicate = - new PersonMatchesKeywordsPredicate(Collections.singletonList("second")); + PersonMatchesSearchPredicate firstPredicate = + new PersonMatchesSearchPredicate(Collections.singletonList("first")); + PersonMatchesSearchPredicate secondPredicate = + new PersonMatchesSearchPredicate(Collections.singletonList("second")); FindCommand findFirstCommand = new FindCommand(firstPredicate); FindCommand findSecondCommand = new FindCommand(secondPredicate); @@ -61,7 +61,7 @@ public void equals() { @Test public void execute_zeroKeywords_noPersonFound() { String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 0); - PersonMatchesKeywordsPredicate predicate = preparePredicate(" "); + PersonMatchesSearchPredicate predicate = preparePredicate(" "); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); @@ -71,7 +71,7 @@ public void execute_zeroKeywords_noPersonFound() { @Test public void execute_multipleKeywords_multiplePersonsFound() { String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 3); - PersonMatchesKeywordsPredicate predicate = preparePredicate("Kurz Elle Kunz"); + PersonMatchesSearchPredicate predicate = preparePredicate("Kurz Elle Kunz"); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); @@ -88,38 +88,16 @@ public void of_emptyArg_throwsParseException() { public void of_validArgs_returnsFindCommand() { // no leading and trailing whitespaces FindCommand expectedFindCommand = - new FindCommand(new PersonMatchesKeywordsPredicate(List.of("Alice", "Bob"))); + new FindCommand(new PersonMatchesSearchPredicate(List.of("Alice", "Bob"))); assertParseSuccess(FindCommand::of, "Alice Bob", expectedFindCommand); // multiple whitespaces between keywords assertParseSuccess(FindCommand::of, " \n Alice \n \t Bob \t", expectedFindCommand); } - @Test - public void execute_fuzzyMatching_partialMatchPersonFound() { - String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 1); - // Assuming "Carl Kurz" is in the address book and "Karl" is a fuzzy match for "Carl" - PersonMatchesKeywordsPredicate predicate = preparePredicate("Karl"); - FindCommand command = new FindCommand(predicate); - expectedModel.updateFilteredPersonList(predicate); - assertCommandSuccess(command, model, expectedMessage, expectedModel); - assertEquals(List.of(CARL), model.getFilteredPersonList()); - } - - @Test - public void execute_fuzzyMatching_misspelledKeywordPersonFound() { - String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 1); - // Assuming "Elle Meyer" is in the address book and "Elee" is a fuzzy match for "Elle" - PersonMatchesKeywordsPredicate predicate = preparePredicate("Elee"); - FindCommand command = new FindCommand(predicate); - expectedModel.updateFilteredPersonList(predicate); - assertCommandSuccess(command, model, expectedMessage, expectedModel); - assertEquals(List.of(ELLE), model.getFilteredPersonList()); - } - @Test public void toStringMethod() { - PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(List.of("keyword")); + PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(List.of("keyword")); FindCommand findCommand = new FindCommand(predicate); String expected = FindCommand.class.getCanonicalName() + "{predicate=" + predicate + "}"; assertEquals(expected, findCommand.toString()); @@ -128,8 +106,8 @@ public void toStringMethod() { /** * Parses {@code userInput} into a {@code NameContainsKeywordsPredicate}. */ - private PersonMatchesKeywordsPredicate preparePredicate(String userInput) { - return new PersonMatchesKeywordsPredicate(List.of(userInput.split("\\s+"))); + private PersonMatchesSearchPredicate preparePredicate(String userInput) { + return new PersonMatchesSearchPredicate(List.of(userInput.split("\\s+"))); } } diff --git a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java index 195aafe9893..c4698a629cb 100644 --- a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java +++ b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java @@ -24,7 +24,7 @@ import seedu.address.logic.commands.ListCommand; import seedu.address.logic.util.exceptions.ParseException; import seedu.address.model.person.Person; -import seedu.address.model.person.PersonMatchesKeywordsPredicate; +import seedu.address.model.person.PersonMatchesSearchPredicate; import seedu.address.testutil.EditPersonDescriptorBuilder; import seedu.address.testutil.PersonBuilder; import seedu.address.testutil.PersonUtil; @@ -71,7 +71,7 @@ public void parseCommand_find() throws Exception { List keywords = Arrays.asList("foo", "bar", "baz"); FindCommand command = (FindCommand) AddressBookParser.parseCommand( FindCommand.COMMAND_WORD + " " + keywords.stream().collect(Collectors.joining(" "))); - assertEquals(new FindCommand(new PersonMatchesKeywordsPredicate(keywords)), command); + assertEquals(new FindCommand(new PersonMatchesSearchPredicate(keywords)), command); } @Test diff --git a/src/test/java/seedu/address/model/ModelManagerTest.java b/src/test/java/seedu/address/model/ModelManagerTest.java index 344e6d544d8..d9dbf737c43 100644 --- a/src/test/java/seedu/address/model/ModelManagerTest.java +++ b/src/test/java/seedu/address/model/ModelManagerTest.java @@ -17,7 +17,7 @@ import seedu.address.commons.core.GuiSettings; import seedu.address.model.exceptions.AddressBookException; -import seedu.address.model.person.PersonMatchesKeywordsPredicate; +import seedu.address.model.person.PersonMatchesSearchPredicate; import seedu.address.testutil.AddressBookBuilder; public class ModelManagerTest { @@ -142,7 +142,7 @@ public void equals() { // different filteredList -> returns false String[] keywords = ALICE.getName().toString().split("\\s+"); - modelManager.updateFilteredPersonList(new PersonMatchesKeywordsPredicate(Arrays.asList(keywords))); + modelManager.updateFilteredPersonList(new PersonMatchesSearchPredicate(Arrays.asList(keywords))); assertFalse(modelManager.equals(new ModelManager(addressBook, userPrefs))); // resets modelManager to initial state for upcoming tests diff --git a/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java b/src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java similarity index 63% rename from src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java rename to src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java index 091f3665be6..2828ddafcd6 100644 --- a/src/test/java/seedu/address/model/person/PersonMatchesKeywordsPredicateTest.java +++ b/src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java @@ -12,24 +12,24 @@ import seedu.address.testutil.PersonBuilder; -public class PersonMatchesKeywordsPredicateTest { +public class PersonMatchesSearchPredicateTest { @Test public void equals() { List firstPredicateKeywordList = Collections.singletonList("first"); List secondPredicateKeywordList = List.of("first", "second"); - PersonMatchesKeywordsPredicate firstPredicate = - new PersonMatchesKeywordsPredicate(firstPredicateKeywordList); - PersonMatchesKeywordsPredicate secondPredicate = - new PersonMatchesKeywordsPredicate(secondPredicateKeywordList); + PersonMatchesSearchPredicate firstPredicate = + new PersonMatchesSearchPredicate(firstPredicateKeywordList); + PersonMatchesSearchPredicate secondPredicate = + new PersonMatchesSearchPredicate(secondPredicateKeywordList); // same object -> returns true assertEquals(firstPredicate, firstPredicate); // same values -> returns true - PersonMatchesKeywordsPredicate firstPredicateCopy = - new PersonMatchesKeywordsPredicate(firstPredicateKeywordList); + PersonMatchesSearchPredicate firstPredicateCopy = + new PersonMatchesSearchPredicate(firstPredicateKeywordList); assertEquals(firstPredicate, firstPredicateCopy); // different types -> returns false @@ -45,39 +45,43 @@ public void equals() { @Test public void test_nameContainsKeywords_returnsTrue() { // One keyword - PersonMatchesKeywordsPredicate predicate = - new PersonMatchesKeywordsPredicate(Collections.singletonList("Alice")); + PersonMatchesSearchPredicate predicate = + new PersonMatchesSearchPredicate(Collections.singletonList("Alice")); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Multiple keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("Alice", "Bob")); + predicate = new PersonMatchesSearchPredicate(List.of("Alice", "Bob")); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Only one matching keyword - predicate = new PersonMatchesKeywordsPredicate(List.of("Bob", "Carol")); + predicate = new PersonMatchesSearchPredicate(List.of("Bob", "Carol")); assertTrue(predicate.test(new PersonBuilder().withName("Alice Carol").build())); // Mixed-case keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("aLIce", "bOB")); + predicate = new PersonMatchesSearchPredicate(List.of("aLIce", "bOB")); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Short query, long name - predicate = new PersonMatchesKeywordsPredicate(List.of("y")); + predicate = new PersonMatchesSearchPredicate(List.of("y")); assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh The Fifth").build())); + + // Query without whitespace + predicate = new PersonMatchesSearchPredicate(List.of("AlexYeoh")); + assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh").build())); } @Test public void test_nameDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(Collections.emptyList()); + PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); assertFalse(predicate.test(new PersonBuilder().withName("Alice").build())); // Non-matching keyword - predicate = new PersonMatchesKeywordsPredicate(List.of("Carol")); + predicate = new PersonMatchesSearchPredicate(List.of("Carol")); assertFalse(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesKeywordsPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") .withEmail("alice@email.com").withAddress("Main Street").build())); } @@ -85,35 +89,35 @@ public void test_nameDoesNotContainKeywords_returnsFalse() { @Test public void test_tagContainsKeywords_returnsTrue() { // One keyword - PersonMatchesKeywordsPredicate predicate = - new PersonMatchesKeywordsPredicate(Collections.singletonList("friends")); + PersonMatchesSearchPredicate predicate = + new PersonMatchesSearchPredicate(Collections.singletonList("friends")); assertTrue(predicate.test(new PersonBuilder().withTags("friends").build())); // Multiple keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("friends", "colleagues")); + predicate = new PersonMatchesSearchPredicate(List.of("friends", "colleagues")); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); // Mixed-case keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("fRieNdS", "cOllEaGuEs")); + predicate = new PersonMatchesSearchPredicate(List.of("fRieNdS", "cOllEaGuEs")); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); // Short query, long tag - predicate = new PersonMatchesKeywordsPredicate(List.of("a")); + predicate = new PersonMatchesSearchPredicate(List.of("a")); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); } @Test public void test_tagDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(Collections.emptyList()); + PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); assertFalse(predicate.test(new PersonBuilder().withTags("friends").build())); // Non-matching keyword - predicate = new PersonMatchesKeywordsPredicate(List.of("friends", "colleagues")); + predicate = new PersonMatchesSearchPredicate(List.of("friends", "colleagues")); assertFalse(predicate.test(new PersonBuilder().withTags("family").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesKeywordsPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") .withEmail("alice@email.com").withAddress("Main Street").withTags("friends").build())); } @@ -121,35 +125,35 @@ public void test_tagDoesNotContainKeywords_returnsFalse() { @Test public void test_assetContainsKeywords_returnsTrue() { // One keyword - PersonMatchesKeywordsPredicate predicate = - new PersonMatchesKeywordsPredicate(Collections.singletonList("hammer")); + PersonMatchesSearchPredicate predicate = + new PersonMatchesSearchPredicate(Collections.singletonList("hammer")); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Multiple keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("hammer", "screwdriver")); + predicate = new PersonMatchesSearchPredicate(List.of("hammer", "screwdriver")); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); // Mixed-case keywords - predicate = new PersonMatchesKeywordsPredicate(List.of("hAmMeR", "sCrEwDriVer")); + predicate = new PersonMatchesSearchPredicate(List.of("hAmMeR", "sCrEwDriVer")); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); // Short query, long asset - predicate = new PersonMatchesKeywordsPredicate(List.of("a")); + predicate = new PersonMatchesSearchPredicate(List.of("a")); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); } @Test public void test_assetDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(Collections.emptyList()); + PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Non-matching keyword - predicate = new PersonMatchesKeywordsPredicate(List.of("helmet", "gloves")); + predicate = new PersonMatchesSearchPredicate(List.of("helmet", "gloves")); assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesKeywordsPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); assertFalse(predicate.test(new PersonBuilder().withName("Alice") .withPhone("12345") .withEmail("alice@email.com") @@ -162,9 +166,9 @@ public void test_assetDoesNotContainKeywords_returnsFalse() { @Test public void toStringMethod() { List keywords = List.of("keyword1", "keyword2"); - PersonMatchesKeywordsPredicate predicate = new PersonMatchesKeywordsPredicate(keywords); + PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(keywords); - String expected = PersonMatchesKeywordsPredicate.class.getCanonicalName() + "{keywords=" + keywords + "}"; + String expected = PersonMatchesSearchPredicate.class.getCanonicalName() + "{keywords=" + keywords + "}"; assertEquals(expected, predicate.toString()); } From e09ba5a3812e7907a126bcb3b8266f7ad724a856 Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:17:56 +0800 Subject: [PATCH 06/13] Change keyword search to simple string search --- docs/UserGuide.md | 32 ++++---- .../address/logic/commands/FindCommand.java | 12 +-- ....java => PersonMatchesQueryPredicate.java} | 24 +++--- .../logic/commands/CommandTestUtil.java | 5 +- .../logic/commands/FindCommandTest.java | 22 +++--- .../logic/util/AddressBookParserTest.java | 12 +-- .../seedu/address/model/ModelManagerTest.java | 7 +- ...a => PersonMatchesQueryPredicateTest.java} | 75 +++++++++---------- 8 files changed, 89 insertions(+), 100 deletions(-) rename src/main/java/seedu/address/model/person/{PersonMatchesSearchPredicate.java => PersonMatchesQueryPredicate.java} (55%) rename src/test/java/seedu/address/model/person/{PersonMatchesSearchPredicateTest.java => PersonMatchesQueryPredicateTest.java} (62%) diff --git a/docs/UserGuide.md b/docs/UserGuide.md index a3f0e18598a..5181c9d31d1 100644 --- a/docs/UserGuide.md +++ b/docs/UserGuide.md @@ -247,14 +247,17 @@ Example: `asset old/hammer new/screwdriver` edits the asset `hammer`, changing i ### Finding Contacts: `find` -Finds contacts whose names, tags or assets contain any of the given keywords. +Finds contacts by names, tags or assets. -Format: `find KEYWORD [KEYWORD]...` +Format: `find QUERY` -Example: `find John` searches all contact names, tags and assets for the keyword `John`. +Example: `find John` searches all contact names, tags and assets for the query `John`. -* At least one keyword must be provided. -* Keywords are case-insensitive. +* The query is case-insensitive. +* Any whitespaces in the query will be removed. +* Any whitespaces in the fields will be removed. +* Each field is individually checked against the query. +* If the query is a substring of the field, then a match is found. -------------------------------------------------------------------------------------------------------------------- @@ -308,17 +311,16 @@ Furthermore, certain edits can cause the AssetBook-3 to behave in unexpected way ## Command summary -Action | Format | Example ------------------|-------------------------------------------------------------------------------|--- -**Add** | `add n/NAME p/PHONE e/EMAIL o/OFFICE [t/TAG]... [a/ASSET]...` | `add n/John Doe e/johndoe@example.com p/+12345678 a/L293D` -**Delete** | `delete INDEX` | `delete 1` -**Edit contact** | `edit INDEX [n/NAME] [p/PHONE] [e/EMAIL] [o/OFFICE] [t/TAG]... [a/ASSET]...` | `edit 1 e/newemail@example.com` -**Edit asset** | `asset old/OLD_ASSET_NAME new/NEW_ASSET_NAME` | `asset old/hammer new/screwdriver` -**Find** | `find KEYWORD [KEYWORD]...` | `find John` -**Undo** | `undo` | `undo` -**Exit** | `exit` | `exit` +Action | Format | Example +-----------------|------------------------------------------------------------------------------|--- +**Add** | `add n/NAME p/PHONE e/EMAIL o/OFFICE [t/TAG]... [a/ASSET]...` | `add n/John Doe e/johndoe@example.com p/+12345678 a/L293D` +**Delete** | `delete INDEX` | `delete 1` +**Edit contact** | `edit INDEX [n/NAME] [p/PHONE] [e/EMAIL] [o/OFFICE] [t/TAG]... [a/ASSET]...` | `edit 1 e/newemail@example.com` +**Edit asset** | `asset old/OLD_ASSET_NAME new/NEW_ASSET_NAME` | `asset old/hammer new/screwdriver` +**Find** | `find QUERY` | `find John` +**Undo** | `undo` | `undo` +**Exit** | `exit` | `exit` ---{.double} ## Glossary - diff --git a/src/main/java/seedu/address/logic/commands/FindCommand.java b/src/main/java/seedu/address/logic/commands/FindCommand.java index 9c5ff03bb7f..aeeecb68a60 100644 --- a/src/main/java/seedu/address/logic/commands/FindCommand.java +++ b/src/main/java/seedu/address/logic/commands/FindCommand.java @@ -3,12 +3,10 @@ import static java.util.Objects.requireNonNull; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; -import java.util.Arrays; - import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.model.Model; -import seedu.address.model.person.PersonMatchesSearchPredicate; +import seedu.address.model.person.PersonMatchesQueryPredicate; /** * Finds and lists all persons in address book whose name contains any of the argument keywords. @@ -24,9 +22,9 @@ public class FindCommand extends Command { + "Parameters: KEYWORD [MORE_KEYWORDS]...\n" + "Example: " + COMMAND_WORD + " alice bob charlie"; - private final PersonMatchesSearchPredicate predicate; + private final PersonMatchesQueryPredicate predicate; - public FindCommand(PersonMatchesSearchPredicate predicate) { + public FindCommand(PersonMatchesQueryPredicate predicate) { this.predicate = predicate; } @@ -49,9 +47,7 @@ public static FindCommand of(String args) throws IllegalArgumentException { String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); } - String[] nameKeywords = trimmedArgs.split("\\s+"); - - return new FindCommand(new PersonMatchesSearchPredicate(Arrays.asList(nameKeywords))); + return new FindCommand(new PersonMatchesQueryPredicate(trimmedArgs)); } @Override diff --git a/src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java similarity index 55% rename from src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java rename to src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index 3ab86d823e3..cefccc3fbbe 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesSearchPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -1,6 +1,5 @@ package seedu.address.model.person; -import java.util.List; import java.util.function.Predicate; import seedu.address.commons.util.ToStringBuilder; @@ -10,21 +9,20 @@ * The algorithm removes all whitespaces and checks if the query is a substring of a field. * The substring matching is case-insensitive. */ -public class PersonMatchesSearchPredicate implements Predicate { +public class PersonMatchesQueryPredicate implements Predicate { - private final List keywords; + private final String query; - public PersonMatchesSearchPredicate(List keywords) { - this.keywords = keywords; + public PersonMatchesQueryPredicate(String query) { + this.query = query; } //@@author rizkidelta @Override public boolean test(Person person) { - return keywords.stream() - .anyMatch(keyword -> substringMatch(person.getName().toString(), keyword) - || person.getTags().stream().anyMatch(tag -> substringMatch(tag.get(), keyword)) - || person.getAssets().stream().anyMatch(asset -> substringMatch(asset.get(), keyword))); + return substringMatch(person.getName().toString(), query) + || person.getTags().stream().anyMatch(tag -> substringMatch(tag.get(), query) + || person.getAssets().stream().anyMatch(asset -> substringMatch(asset.get(), query))); } //@@author rizkidelta @@ -46,17 +44,17 @@ public boolean equals(Object other) { } // instanceof handles nulls - if (!(other instanceof PersonMatchesSearchPredicate)) { + if (!(other instanceof PersonMatchesQueryPredicate)) { return false; } - PersonMatchesSearchPredicate otherPersonMatchesSearchPredicate = (PersonMatchesSearchPredicate) other; - return keywords.equals(otherPersonMatchesSearchPredicate.keywords); + PersonMatchesQueryPredicate otherPersonMatchesQueryPredicate = (PersonMatchesQueryPredicate) other; + return query.equals(otherPersonMatchesQueryPredicate.query); } @Override public String toString() { - return new ToStringBuilder(this).add("keywords", keywords).toString(); + return new ToStringBuilder(this).add("keywords", query).toString(); } } diff --git a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java index 61965d43cb5..0f640163ef0 100644 --- a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java +++ b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java @@ -11,7 +11,6 @@ import static seedu.address.testutil.Assert.assertThrows; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import seedu.address.commons.core.index.Index; @@ -19,7 +18,7 @@ import seedu.address.model.AddressBook; import seedu.address.model.Model; import seedu.address.model.person.Person; -import seedu.address.model.person.PersonMatchesSearchPredicate; +import seedu.address.model.person.PersonMatchesQueryPredicate; import seedu.address.testutil.EditPersonDescriptorBuilder; /** @@ -118,7 +117,7 @@ public static void showPersonAtIndex(Model model, Index targetIndex) { Person person = model.getFilteredPersonList().get(targetIndex.getZeroBased()); final String[] splitName = person.getName().toString().split("\\s+"); - model.updateFilteredPersonList(new PersonMatchesSearchPredicate(Arrays.asList(splitName[0]))); + model.updateFilteredPersonList(new PersonMatchesQueryPredicate(splitName[0])); assertEquals(1, model.getFilteredPersonList().size()); } diff --git a/src/test/java/seedu/address/logic/commands/FindCommandTest.java b/src/test/java/seedu/address/logic/commands/FindCommandTest.java index 0129185d6e5..99f6ce8be6f 100644 --- a/src/test/java/seedu/address/logic/commands/FindCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/FindCommandTest.java @@ -21,7 +21,7 @@ import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; -import seedu.address.model.person.PersonMatchesSearchPredicate; +import seedu.address.model.person.PersonMatchesQueryPredicate; /** * Contains integration tests (interaction with the Model) for {@code FindCommand}. @@ -33,10 +33,10 @@ public class FindCommandTest { @Test public void equals() { - PersonMatchesSearchPredicate firstPredicate = - new PersonMatchesSearchPredicate(Collections.singletonList("first")); - PersonMatchesSearchPredicate secondPredicate = - new PersonMatchesSearchPredicate(Collections.singletonList("second")); + PersonMatchesQueryPredicate firstPredicate = + new PersonMatchesQueryPredicate("first"); + PersonMatchesQueryPredicate secondPredicate = + new PersonMatchesQueryPredicate("second"); FindCommand findFirstCommand = new FindCommand(firstPredicate); FindCommand findSecondCommand = new FindCommand(secondPredicate); @@ -61,7 +61,7 @@ public void equals() { @Test public void execute_zeroKeywords_noPersonFound() { String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 0); - PersonMatchesSearchPredicate predicate = preparePredicate(" "); + PersonMatchesQueryPredicate predicate = preparePredicate(" "); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); @@ -71,7 +71,7 @@ public void execute_zeroKeywords_noPersonFound() { @Test public void execute_multipleKeywords_multiplePersonsFound() { String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 3); - PersonMatchesSearchPredicate predicate = preparePredicate("Kurz Elle Kunz"); + PersonMatchesQueryPredicate predicate = preparePredicate("Kurz Elle Kunz"); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); @@ -88,7 +88,7 @@ public void of_emptyArg_throwsParseException() { public void of_validArgs_returnsFindCommand() { // no leading and trailing whitespaces FindCommand expectedFindCommand = - new FindCommand(new PersonMatchesSearchPredicate(List.of("Alice", "Bob"))); + new FindCommand(new PersonMatchesQueryPredicate("Alice Bob")); assertParseSuccess(FindCommand::of, "Alice Bob", expectedFindCommand); // multiple whitespaces between keywords @@ -97,7 +97,7 @@ public void of_validArgs_returnsFindCommand() { @Test public void toStringMethod() { - PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(List.of("keyword")); + PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate("keyword"); FindCommand findCommand = new FindCommand(predicate); String expected = FindCommand.class.getCanonicalName() + "{predicate=" + predicate + "}"; assertEquals(expected, findCommand.toString()); @@ -106,8 +106,8 @@ public void toStringMethod() { /** * Parses {@code userInput} into a {@code NameContainsKeywordsPredicate}. */ - private PersonMatchesSearchPredicate preparePredicate(String userInput) { - return new PersonMatchesSearchPredicate(List.of(userInput.split("\\s+"))); + private PersonMatchesQueryPredicate preparePredicate(String userInput) { + return new PersonMatchesQueryPredicate(userInput); } } diff --git a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java index c4698a629cb..6afd03bed23 100644 --- a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java +++ b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java @@ -7,10 +7,6 @@ import static seedu.address.testutil.Assert.assertThrows; import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON; -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; - import org.junit.jupiter.api.Test; import seedu.address.logic.commands.AddCommand; @@ -24,7 +20,7 @@ import seedu.address.logic.commands.ListCommand; import seedu.address.logic.util.exceptions.ParseException; import seedu.address.model.person.Person; -import seedu.address.model.person.PersonMatchesSearchPredicate; +import seedu.address.model.person.PersonMatchesQueryPredicate; import seedu.address.testutil.EditPersonDescriptorBuilder; import seedu.address.testutil.PersonBuilder; import seedu.address.testutil.PersonUtil; @@ -68,10 +64,10 @@ public void parseCommand_exit() throws Exception { @Test public void parseCommand_find() throws Exception { - List keywords = Arrays.asList("foo", "bar", "baz"); + String query = "foo bar baz"; FindCommand command = (FindCommand) AddressBookParser.parseCommand( - FindCommand.COMMAND_WORD + " " + keywords.stream().collect(Collectors.joining(" "))); - assertEquals(new FindCommand(new PersonMatchesSearchPredicate(keywords)), command); + FindCommand.COMMAND_WORD + " " + query); + assertEquals(new FindCommand(new PersonMatchesQueryPredicate(query)), command); } @Test diff --git a/src/test/java/seedu/address/model/ModelManagerTest.java b/src/test/java/seedu/address/model/ModelManagerTest.java index d9dbf737c43..6d44b238344 100644 --- a/src/test/java/seedu/address/model/ModelManagerTest.java +++ b/src/test/java/seedu/address/model/ModelManagerTest.java @@ -11,13 +11,12 @@ import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import org.junit.jupiter.api.Test; import seedu.address.commons.core.GuiSettings; import seedu.address.model.exceptions.AddressBookException; -import seedu.address.model.person.PersonMatchesSearchPredicate; +import seedu.address.model.person.PersonMatchesQueryPredicate; import seedu.address.testutil.AddressBookBuilder; public class ModelManagerTest { @@ -141,8 +140,8 @@ public void equals() { assertFalse(modelManager.equals(new ModelManager(differentAddressBook, userPrefs))); // different filteredList -> returns false - String[] keywords = ALICE.getName().toString().split("\\s+"); - modelManager.updateFilteredPersonList(new PersonMatchesSearchPredicate(Arrays.asList(keywords))); + String query = ALICE.getName().toString(); + modelManager.updateFilteredPersonList(new PersonMatchesQueryPredicate(query)); assertFalse(modelManager.equals(new ModelManager(addressBook, userPrefs))); // resets modelManager to initial state for upcoming tests diff --git a/src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java similarity index 62% rename from src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java rename to src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java index 2828ddafcd6..d0fb1e53688 100644 --- a/src/test/java/seedu/address/model/person/PersonMatchesSearchPredicateTest.java +++ b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java @@ -5,31 +5,30 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.Test; import seedu.address.testutil.PersonBuilder; -public class PersonMatchesSearchPredicateTest { +public class PersonMatchesQueryPredicateTest { @Test public void equals() { - List firstPredicateKeywordList = Collections.singletonList("first"); - List secondPredicateKeywordList = List.of("first", "second"); + String firstPredicateQuery = "first"; + String secondPredicateQuery = "first second"; - PersonMatchesSearchPredicate firstPredicate = - new PersonMatchesSearchPredicate(firstPredicateKeywordList); - PersonMatchesSearchPredicate secondPredicate = - new PersonMatchesSearchPredicate(secondPredicateKeywordList); + PersonMatchesQueryPredicate firstPredicate = + new PersonMatchesQueryPredicate(firstPredicateQuery); + PersonMatchesQueryPredicate secondPredicate = + new PersonMatchesQueryPredicate(secondPredicateQuery); // same object -> returns true assertEquals(firstPredicate, firstPredicate); // same values -> returns true - PersonMatchesSearchPredicate firstPredicateCopy = - new PersonMatchesSearchPredicate(firstPredicateKeywordList); + PersonMatchesQueryPredicate firstPredicateCopy = + new PersonMatchesQueryPredicate(firstPredicateQuery); assertEquals(firstPredicate, firstPredicateCopy); // different types -> returns false @@ -45,43 +44,43 @@ public void equals() { @Test public void test_nameContainsKeywords_returnsTrue() { // One keyword - PersonMatchesSearchPredicate predicate = - new PersonMatchesSearchPredicate(Collections.singletonList("Alice")); + PersonMatchesQueryPredicate predicate = + new PersonMatchesQueryPredicate("Alice"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Multiple keywords - predicate = new PersonMatchesSearchPredicate(List.of("Alice", "Bob")); + predicate = new PersonMatchesQueryPredicate("Alice Bob"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Only one matching keyword - predicate = new PersonMatchesSearchPredicate(List.of("Bob", "Carol")); + predicate = new PersonMatchesQueryPredicate("Bob Carol"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Carol").build())); // Mixed-case keywords - predicate = new PersonMatchesSearchPredicate(List.of("aLIce", "bOB")); + predicate = new PersonMatchesQueryPredicate("aLIce bOB"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Short query, long name - predicate = new PersonMatchesSearchPredicate(List.of("y")); + predicate = new PersonMatchesQueryPredicate("y"); assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh The Fifth").build())); // Query without whitespace - predicate = new PersonMatchesSearchPredicate(List.of("AlexYeoh")); + predicate = new PersonMatchesQueryPredicate("AlexYeoh"); assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh").build())); } @Test public void test_nameDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); + PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); assertFalse(predicate.test(new PersonBuilder().withName("Alice").build())); // Non-matching keyword - predicate = new PersonMatchesSearchPredicate(List.of("Carol")); + predicate = new PersonMatchesQueryPredicate("Carol"); assertFalse(predicate.test(new PersonBuilder().withName("Alice Bob").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") .withEmail("alice@email.com").withAddress("Main Street").build())); } @@ -89,35 +88,35 @@ public void test_nameDoesNotContainKeywords_returnsFalse() { @Test public void test_tagContainsKeywords_returnsTrue() { // One keyword - PersonMatchesSearchPredicate predicate = - new PersonMatchesSearchPredicate(Collections.singletonList("friends")); + PersonMatchesQueryPredicate predicate = + new PersonMatchesQueryPredicate("friends"); assertTrue(predicate.test(new PersonBuilder().withTags("friends").build())); // Multiple keywords - predicate = new PersonMatchesSearchPredicate(List.of("friends", "colleagues")); + predicate = new PersonMatchesQueryPredicate("friends colleagues"); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); // Mixed-case keywords - predicate = new PersonMatchesSearchPredicate(List.of("fRieNdS", "cOllEaGuEs")); + predicate = new PersonMatchesQueryPredicate("fRieNdS cOllEaGuEs"); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); // Short query, long tag - predicate = new PersonMatchesSearchPredicate(List.of("a")); + predicate = new PersonMatchesQueryPredicate("a"); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); } @Test public void test_tagDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); + PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); assertFalse(predicate.test(new PersonBuilder().withTags("friends").build())); // Non-matching keyword - predicate = new PersonMatchesSearchPredicate(List.of("friends", "colleagues")); + predicate = new PersonMatchesQueryPredicate("friends colleagues"); assertFalse(predicate.test(new PersonBuilder().withTags("family").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") .withEmail("alice@email.com").withAddress("Main Street").withTags("friends").build())); } @@ -125,35 +124,35 @@ public void test_tagDoesNotContainKeywords_returnsFalse() { @Test public void test_assetContainsKeywords_returnsTrue() { // One keyword - PersonMatchesSearchPredicate predicate = - new PersonMatchesSearchPredicate(Collections.singletonList("hammer")); + PersonMatchesQueryPredicate predicate = + new PersonMatchesQueryPredicate("hammer"); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Multiple keywords - predicate = new PersonMatchesSearchPredicate(List.of("hammer", "screwdriver")); + predicate = new PersonMatchesQueryPredicate("hammer screwdriver"); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); // Mixed-case keywords - predicate = new PersonMatchesSearchPredicate(List.of("hAmMeR", "sCrEwDriVer")); + predicate = new PersonMatchesQueryPredicate("hAmMeR sCrEwDriVer"); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); // Short query, long asset - predicate = new PersonMatchesSearchPredicate(List.of("a")); + predicate = new PersonMatchesQueryPredicate("a"); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); } @Test public void test_assetDoesNotContainKeywords_returnsFalse() { // Zero keywords - PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(Collections.emptyList()); + PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Non-matching keyword - predicate = new PersonMatchesSearchPredicate(List.of("helmet", "gloves")); + predicate = new PersonMatchesQueryPredicate("helmet gloves"); assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesSearchPredicate(List.of("12345", "alice@email.com", "Main", "Street")); + predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); assertFalse(predicate.test(new PersonBuilder().withName("Alice") .withPhone("12345") .withEmail("alice@email.com") @@ -166,9 +165,9 @@ public void test_assetDoesNotContainKeywords_returnsFalse() { @Test public void toStringMethod() { List keywords = List.of("keyword1", "keyword2"); - PersonMatchesSearchPredicate predicate = new PersonMatchesSearchPredicate(keywords); + PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); - String expected = PersonMatchesSearchPredicate.class.getCanonicalName() + "{keywords=" + keywords + "}"; + String expected = PersonMatchesQueryPredicate.class.getCanonicalName() + "{keywords=" + keywords + "}"; assertEquals(expected, predicate.toString()); } From 3623bd4d42dedc635314917c2b6c6d65e0c1a0c2 Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:20:33 +0800 Subject: [PATCH 07/13] Cleanup build.gradle --- build.gradle | 2 -- 1 file changed, 2 deletions(-) diff --git a/build.gradle b/build.gradle index 3cbf3339c31..a378812b244 100644 --- a/build.gradle +++ b/build.gradle @@ -60,8 +60,6 @@ dependencies { implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.7.0' implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.7.4' -// implementation group: 'com.productiveedge', name: 'fuzzy-search', version: '1.0' - testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: jUnitVersion testRuntimeOnly group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: jUnitVersion From 7c9c74a876c1f27f7861f171be747d16dc0f7abe Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 23:19:56 +0800 Subject: [PATCH 08/13] Make explanation in UG clearer --- docs/UserGuide.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/UserGuide.md b/docs/UserGuide.md index 5181c9d31d1..292d378fb13 100644 --- a/docs/UserGuide.md +++ b/docs/UserGuide.md @@ -254,10 +254,9 @@ Format: `find QUERY` Example: `find John` searches all contact names, tags and assets for the query `John`. * The query is case-insensitive. -* Any whitespaces in the query will be removed. -* Any whitespaces in the fields will be removed. +* All whitespaces in both the query and fields will be ignored. * Each field is individually checked against the query. -* If the query is a substring of the field, then a match is found. +* A match is found if the query is a substring of the field being checked. -------------------------------------------------------------------------------------------------------------------- From 19465cccdce7c5c71c02b341fa7fb0a39accd57b Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Wed, 3 Apr 2024 23:58:04 +0800 Subject: [PATCH 09/13] Implement person predicate stub in FindCommandTest --- .../address/logic/commands/FindCommand.java | 28 ++++---- .../person/PersonMatchesQueryPredicate.java | 27 +++----- .../logic/commands/FindCommandTest.java | 67 ++++++++++++------- .../PersonMatchesQueryPredicateTest.java | 11 --- 4 files changed, 65 insertions(+), 68 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/FindCommand.java b/src/main/java/seedu/address/logic/commands/FindCommand.java index aeeecb68a60..e8da4da328a 100644 --- a/src/main/java/seedu/address/logic/commands/FindCommand.java +++ b/src/main/java/seedu/address/logic/commands/FindCommand.java @@ -3,28 +3,31 @@ import static java.util.Objects.requireNonNull; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; -import seedu.address.commons.util.ToStringBuilder; +import java.util.function.Predicate; + import seedu.address.logic.Messages; import seedu.address.model.Model; +import seedu.address.model.person.Person; import seedu.address.model.person.PersonMatchesQueryPredicate; /** - * 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 the address book such that certain fields match the predicate, + * See {@code PersonMatchesQueryPredicate}. */ 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, assets or tags 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"; + + ": Finds all persons whose names, assets or tags contains " + + "the specified query (case-insensitive) and displays them as a list with index numbers.\n" + + "All whitespaces are ignored.\n" + + "Parameters: QUERY\n" + + "Example: " + COMMAND_WORD + " alex"; - private final PersonMatchesQueryPredicate predicate; + private final Predicate predicate; - public FindCommand(PersonMatchesQueryPredicate predicate) { + public FindCommand(Predicate predicate) { this.predicate = predicate; } @@ -65,11 +68,4 @@ public boolean equals(Object other) { return predicate.equals(otherFindCommand.predicate); } - @Override - public String toString() { - return new ToStringBuilder(this) - .add("predicate", predicate) - .toString(); - } - } diff --git a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index cefccc3fbbe..342e5259c6d 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -2,8 +2,6 @@ import java.util.function.Predicate; -import seedu.address.commons.util.ToStringBuilder; - /** * Tests that a {@code Person}'s {@code Name}, {@code Tags} or {@code Assets} matches the search query. * The algorithm removes all whitespaces and checks if the query is a substring of a field. @@ -11,10 +9,17 @@ */ public class PersonMatchesQueryPredicate implements Predicate { + private static final String whitespaceRegex = "\\s+"; + private static final String emptyString = ""; + private final String query; public PersonMatchesQueryPredicate(String query) { - this.query = query; + this.query = processString(query); + } + + private static String processString(String str) { + return str.toLowerCase().replaceAll(whitespaceRegex, emptyString); } //@@author rizkidelta @@ -26,14 +31,9 @@ public boolean test(Person person) { } //@@author rizkidelta - private boolean substringMatch(String text, String query) { - final String whitespaceRegex = "\\s+"; - final String emptyString = ""; - - // remove all whitespaces from strings - text = text.toLowerCase().replaceAll(whitespaceRegex, emptyString); - query = query.toLowerCase().replaceAll(whitespaceRegex, emptyString); - + private static boolean substringMatch(String text, String query) { + // only need to process text, as query is already processed in the constructor + text = processString(text); return text.contains(query); } @@ -52,9 +52,4 @@ public boolean equals(Object other) { return query.equals(otherPersonMatchesQueryPredicate.query); } - @Override - public String toString() { - return new ToStringBuilder(this).add("keywords", query).toString(); - } - } diff --git a/src/test/java/seedu/address/logic/commands/FindCommandTest.java b/src/test/java/seedu/address/logic/commands/FindCommandTest.java index 618d648ba23..d995e14eed0 100644 --- a/src/test/java/seedu/address/logic/commands/FindCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/FindCommandTest.java @@ -8,19 +8,18 @@ import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess; import static seedu.address.logic.commands.CommandTestUtil.assertParseFailure; import static seedu.address.logic.commands.CommandTestUtil.assertParseSuccess; -import static seedu.address.testutil.TypicalPersons.CARL; -import static seedu.address.testutil.TypicalPersons.ELLE; -import static seedu.address.testutil.TypicalPersons.FIONA; import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook; +import static seedu.address.testutil.TypicalPersons.getTypicalPersons; -import java.util.Collections; import java.util.List; +import java.util.function.Predicate; import org.junit.jupiter.api.Test; import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; +import seedu.address.model.person.Person; import seedu.address.model.person.PersonMatchesQueryPredicate; /** @@ -30,13 +29,12 @@ public class FindCommandTest { private final Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs()); private final Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs()); + private final List personsList = getTypicalPersons(); @Test public void equals() { - PersonMatchesQueryPredicate firstPredicate = - new PersonMatchesQueryPredicate("first"); - PersonMatchesQueryPredicate secondPredicate = - new PersonMatchesQueryPredicate("second"); + Predicate firstPredicate = new PersonPredicateStub(false); + Predicate secondPredicate = new PersonPredicateStub(true); FindCommand findFirstCommand = new FindCommand(firstPredicate); FindCommand findSecondCommand = new FindCommand(secondPredicate); @@ -59,23 +57,23 @@ public void equals() { } @Test - public void execute_zeroKeywords_noPersonFound() { + public void execute_stubThatReturnsFalse_noPersonFound() { String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 0); - PersonMatchesQueryPredicate predicate = preparePredicate(" "); + Predicate predicate = new PersonPredicateStub(false); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); - assertEquals(Collections.emptyList(), model.getFilteredPersonList()); + assertEquals(List.of(), model.getFilteredPersonList()); } @Test - public void execute_multipleKeywords_multiplePersonsFound() { - String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, 3); - PersonMatchesQueryPredicate predicate = preparePredicate("Kurz Elle Kunz"); + public void execute_stubThatReturnsTrue_allPersonsFound() { + String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, personsList.size()); + PersonPredicateStub predicate = new PersonPredicateStub(true); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); assertCommandSuccess(command, model, expectedMessage, expectedModel); - assertEquals(List.of(CARL, ELLE, FIONA), model.getFilteredPersonList()); + assertEquals(personsList, model.getFilteredPersonList()); } @Test @@ -95,19 +93,38 @@ public void of_validArgs_returnsFindCommand() { assertParseSuccess(FindCommand::of, " \n Alice \n \t Bob \t", expectedFindCommand); } - @Test - public void toStringMethod() { - PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate("keyword"); - FindCommand findCommand = new FindCommand(predicate); - String expected = FindCommand.class.getCanonicalName() + "{predicate=" + predicate + "}"; - assertEquals(expected, findCommand.toString()); - } /** - * Parses {@code userInput} into a {@code NameContainsKeywordsPredicate}. + * A predicate stub class for testing the FindCommand class. */ - private PersonMatchesQueryPredicate preparePredicate(String userInput) { - return new PersonMatchesQueryPredicate(userInput); + private static class PersonPredicateStub implements Predicate { + + private final boolean returnValue; + + public PersonPredicateStub(boolean returnValue) { + this.returnValue = returnValue; + } + + @Override + public boolean test(Person person) { + return returnValue; + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + + // instanceof handles nulls + if (!(other instanceof PersonPredicateStub)) { + return false; + } + + PersonPredicateStub otherFindCommand = (PersonPredicateStub) other; + return returnValue == otherFindCommand.returnValue; + } + } } diff --git a/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java index 1943190b5f2..ec6f5510db7 100644 --- a/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java +++ b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java @@ -4,8 +4,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.List; - import org.junit.jupiter.api.Test; import seedu.address.testutil.PersonBuilder; @@ -161,13 +159,4 @@ public void test_assetDoesNotContainKeywords_returnsFalse() { .build())); } - @Test - public void toStringMethod() { - List keywords = List.of("keyword1", "keyword2"); - PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); - - String expected = PersonMatchesQueryPredicate.class.getCanonicalName() + "{keywords=" + keywords + "}"; - assertEquals(expected, predicate.toString()); - } - } From 31b867f780bbad1e05160646daba535cf4fef6d8 Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Thu, 4 Apr 2024 00:36:03 +0800 Subject: [PATCH 10/13] Fix testcases --- .../person/PersonMatchesQueryPredicate.java | 10 ++ .../java/seedu/address/model/tag/Tag.java | 2 +- .../logic/commands/FindCommandTest.java | 12 +- .../PersonMatchesQueryPredicateTest.java | 152 +++++++++--------- 4 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index 342e5259c6d..4fb98edb54c 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -1,5 +1,7 @@ package seedu.address.model.person; +import static java.util.Objects.requireNonNull; + import java.util.function.Predicate; /** @@ -14,7 +16,15 @@ public class PersonMatchesQueryPredicate implements Predicate { private final String query; + /** + * Processes the query string and constructs the object. + * @param query the query string. + * @throws NullPointerException if the query string is null. + * @throws AssertionError if the query string is empty. + */ public PersonMatchesQueryPredicate(String query) { + requireNonNull(query); + assert !query.isEmpty(); this.query = processString(query); } diff --git a/src/main/java/seedu/address/model/tag/Tag.java b/src/main/java/seedu/address/model/tag/Tag.java index c90c31cea83..9cc74c61c15 100644 --- a/src/main/java/seedu/address/model/tag/Tag.java +++ b/src/main/java/seedu/address/model/tag/Tag.java @@ -7,7 +7,7 @@ /** * Represents a Tag in the address book. - * Guarantees: immutable; name is valid as declared in {@link #isValidTagName(String)} + * Guarantees: immutable; name is valid as declared in {@link #isValid(String)} */ public class Tag { diff --git a/src/test/java/seedu/address/logic/commands/FindCommandTest.java b/src/test/java/seedu/address/logic/commands/FindCommandTest.java index d995e14eed0..deca26795a8 100644 --- a/src/test/java/seedu/address/logic/commands/FindCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/FindCommandTest.java @@ -68,10 +68,11 @@ public void execute_stubThatReturnsFalse_noPersonFound() { @Test public void execute_stubThatReturnsTrue_allPersonsFound() { - String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, personsList.size()); PersonPredicateStub predicate = new PersonPredicateStub(true); FindCommand command = new FindCommand(predicate); expectedModel.updateFilteredPersonList(predicate); + + String expectedMessage = String.format(MESSAGE_PERSONS_LISTED_OVERVIEW, personsList.size()); assertCommandSuccess(command, model, expectedMessage, expectedModel); assertEquals(personsList, model.getFilteredPersonList()); } @@ -84,18 +85,19 @@ public void of_emptyArg_throwsParseException() { @Test public void of_validArgs_returnsFindCommand() { + // this test depends on the PersonMatchesQueryPredicate class in order to test the factory constructor of() + // no leading and trailing whitespaces - FindCommand expectedFindCommand = - new FindCommand(new PersonMatchesQueryPredicate("Alice Bob")); + FindCommand expectedFindCommand = new FindCommand(new PersonMatchesQueryPredicate("Alice Bob")); assertParseSuccess(FindCommand::of, "Alice Bob", expectedFindCommand); // multiple whitespaces between keywords assertParseSuccess(FindCommand::of, " \n Alice \n \t Bob \t", expectedFindCommand); } - /** - * A predicate stub class for testing the FindCommand class. + * Represents a predicate stub class for testing the FindCommand class. + * Removes the dependency on the PersonMatchesQueryPredicate class. */ private static class PersonPredicateStub implements Predicate { diff --git a/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java index ec6f5510db7..d2a4bfa0799 100644 --- a/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java +++ b/src/test/java/seedu/address/model/person/PersonMatchesQueryPredicateTest.java @@ -2,30 +2,32 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.function.Predicate; + import org.junit.jupiter.api.Test; import seedu.address.testutil.PersonBuilder; public class PersonMatchesQueryPredicateTest { + private Predicate predicate; + @Test public void equals() { String firstPredicateQuery = "first"; - String secondPredicateQuery = "first second"; + String secondPredicateQuery = "second"; - PersonMatchesQueryPredicate firstPredicate = - new PersonMatchesQueryPredicate(firstPredicateQuery); - PersonMatchesQueryPredicate secondPredicate = - new PersonMatchesQueryPredicate(secondPredicateQuery); + Predicate firstPredicate = new PersonMatchesQueryPredicate(firstPredicateQuery); + Predicate secondPredicate = new PersonMatchesQueryPredicate(secondPredicateQuery); // same object -> returns true assertEquals(firstPredicate, firstPredicate); // same values -> returns true - PersonMatchesQueryPredicate firstPredicateCopy = - new PersonMatchesQueryPredicate(firstPredicateQuery); + Predicate firstPredicateCopy = new PersonMatchesQueryPredicate(firstPredicateQuery); assertEquals(firstPredicate, firstPredicateCopy); // different types -> returns false @@ -39,21 +41,16 @@ public void equals() { } @Test - public void test_nameContainsKeywords_returnsTrue() { - // One keyword - PersonMatchesQueryPredicate predicate = - new PersonMatchesQueryPredicate("Alice"); + public void test_nameMatchesQuery_returnsTrue() { + // Partial match + predicate = new PersonMatchesQueryPredicate("Ali"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); - // Multiple keywords + // Exact string match predicate = new PersonMatchesQueryPredicate("Alice Bob"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); - // Only one matching keyword - predicate = new PersonMatchesQueryPredicate("Bob Carol"); - assertTrue(predicate.test(new PersonBuilder().withName("Alice Carol").build())); - - // Mixed-case keywords + // Mixed-case string match predicate = new PersonMatchesQueryPredicate("aLIce bOB"); assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build())); @@ -62,39 +59,62 @@ public void test_nameContainsKeywords_returnsTrue() { assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh The Fifth").build())); // Query without whitespace - predicate = new PersonMatchesQueryPredicate("AlexYeoh"); + predicate = new PersonMatchesQueryPredicate("xy"); assertTrue(predicate.test(new PersonBuilder().withName("Alex Yeoh").build())); } @Test - public void test_nameDoesNotContainKeywords_returnsFalse() { - // Zero keywords - PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); - assertFalse(predicate.test(new PersonBuilder().withName("Alice").build())); + public void test_invalidString_throwsException() { + // Empty string should throw an exception + assertThrows(AssertionError.class, () -> new PersonMatchesQueryPredicate("")); + + // Null input should throw an exception + assertThrows(NullPointerException.class, () -> new PersonMatchesQueryPredicate(null)); + } + + @Test + public void test_fieldsDoNotMatchQuery_returnsFalse() { + // Query matches email and address, but does not match name, tags or assets + predicate = new PersonMatchesQueryPredicate("a"); + assertFalse(predicate.test(new PersonBuilder().withName("Zed") + .withEmail("alice@email.com") + .withAddress("Main Street") + .withTags("friends") + .withAssets("screwdriver") + .build())); + + // Query matches phone, but does not match name, tags or assets + predicate = new PersonMatchesQueryPredicate("1"); + assertFalse(predicate.test(new PersonBuilder().withName("Zed") + .withPhone("12345") + .withTags("friends") + .withAssets("screwdriver") + .build())); + } + + @Test + public void test_nameDoesNotMatchQuery_returnsFalse() { + // Only one matching word + predicate = new PersonMatchesQueryPredicate("Bob Carol"); + assertFalse(predicate.test(new PersonBuilder().withName("Alice Carol").build())); - // Non-matching keyword + // Non-matching word predicate = new PersonMatchesQueryPredicate("Carol"); assertFalse(predicate.test(new PersonBuilder().withName("Alice Bob").build())); - - // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); - assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") - .withEmail("alice@email.com").withAddress("Main Street").build())); } @Test - public void test_tagContainsKeywords_returnsTrue() { - // One keyword - PersonMatchesQueryPredicate predicate = - new PersonMatchesQueryPredicate("friends"); + public void test_tagMatchesQuery_returnsTrue() { + // Exact string match + predicate = new PersonMatchesQueryPredicate("friends"); assertTrue(predicate.test(new PersonBuilder().withTags("friends").build())); - // Multiple keywords - predicate = new PersonMatchesQueryPredicate("friends colleagues"); + // Partial match + predicate = new PersonMatchesQueryPredicate("coll"); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); - // Mixed-case keywords - predicate = new PersonMatchesQueryPredicate("fRieNdS cOllEaGuEs"); + // Mixed-case match + predicate = new PersonMatchesQueryPredicate("cOllEaGuEs"); assertTrue(predicate.test(new PersonBuilder().withTags("colleagues").build())); // Short query, long tag @@ -103,60 +123,44 @@ public void test_tagContainsKeywords_returnsTrue() { } @Test - public void test_tagDoesNotContainKeywords_returnsFalse() { - // Zero keywords - PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); - assertFalse(predicate.test(new PersonBuilder().withTags("friends").build())); - - // Non-matching keyword - predicate = new PersonMatchesQueryPredicate("friends colleagues"); + public void test_tagDoesNotMatchQuery_returnsFalse() { + // Non-matching string + predicate = new PersonMatchesQueryPredicate("femily"); assertFalse(predicate.test(new PersonBuilder().withTags("family").build())); - - // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); - assertFalse(predicate.test(new PersonBuilder().withName("Alice").withPhone("12345") - .withEmail("alice@email.com").withAddress("Main Street").withTags("friends").build())); } @Test - public void test_assetContainsKeywords_returnsTrue() { - // One keyword - PersonMatchesQueryPredicate predicate = - new PersonMatchesQueryPredicate("hammer"); + public void test_assetMatchesQuery_returnsTrue() { + // Exact string match + predicate = new PersonMatchesQueryPredicate("hammer"); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); - // Multiple keywords - predicate = new PersonMatchesQueryPredicate("hammer screwdriver"); + // Partial string match + predicate = new PersonMatchesQueryPredicate("driver"); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); - // Mixed-case keywords - predicate = new PersonMatchesQueryPredicate("hAmMeR sCrEwDriVer"); + // Mixed-case match + predicate = new PersonMatchesQueryPredicate("sCrEwDriVer"); assertTrue(predicate.test(new PersonBuilder().withAssets("screwdriver").build())); // Short query, long asset predicate = new PersonMatchesQueryPredicate("a"); assertTrue(predicate.test(new PersonBuilder().withAssets("hammer").build())); + + // Query without whitespace + predicate = new PersonMatchesQueryPredicate("rsc"); + assertTrue(predicate.test(new PersonBuilder().withAssets("hammer screw").build())); } @Test - public void test_assetDoesNotContainKeywords_returnsFalse() { - // Zero keywords - PersonMatchesQueryPredicate predicate = new PersonMatchesQueryPredicate(""); - assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); - - // Non-matching keyword - predicate = new PersonMatchesQueryPredicate("helmet gloves"); - assertFalse(predicate.test(new PersonBuilder().withAssets("hammer").build())); - - // Keywords match phone, email and address, but does not match name, tags or assets - predicate = new PersonMatchesQueryPredicate("12345 alice@email.com Main Street"); - assertFalse(predicate.test(new PersonBuilder().withName("Alice") - .withPhone("12345") - .withEmail("alice@email.com") - .withAddress("Main Street") - .withTags("friends") - .withAssets("hammer") - .build())); + public void test_assetDoesNotMatchQuery_returnsFalse() { + // Non-matching query + predicate = new PersonMatchesQueryPredicate("halmet"); + assertFalse(predicate.test(new PersonBuilder().withAssets("helmet").build())); + + // Search query longer than field size + predicate = new PersonMatchesQueryPredicate("helmet screwdriver"); + assertFalse(predicate.test(new PersonBuilder().withAssets("helmet").build())); } } From 92daf4adda81a2f1bbcaa318a4a31d61fc2cf334 Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Thu, 4 Apr 2024 00:42:32 +0800 Subject: [PATCH 11/13] Rename boolean method to sound like boolean --- .../address/model/person/PersonMatchesQueryPredicate.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index 4fb98edb54c..10948c178d7 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -35,13 +35,13 @@ private static String processString(String str) { //@@author rizkidelta @Override public boolean test(Person person) { - return substringMatch(person.getName().toString(), query) - || person.getTags().stream().anyMatch(tag -> substringMatch(tag.get(), query) - || person.getAssets().stream().anyMatch(asset -> substringMatch(asset.get(), query))); + return doesStringMatchQuery(person.getName().toString(), query) + || person.getTags().stream().anyMatch(tag -> doesStringMatchQuery(tag.get(), query) + || person.getAssets().stream().anyMatch(asset -> doesStringMatchQuery(asset.get(), query))); } //@@author rizkidelta - private static boolean substringMatch(String text, String query) { + private static boolean doesStringMatchQuery(String text, String query) { // only need to process text, as query is already processed in the constructor text = processString(text); return text.contains(query); From a5dc75a49c5c61757c6c546cc444cdb4d5acba2d Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Thu, 4 Apr 2024 00:44:02 +0800 Subject: [PATCH 12/13] Add requireNonNull check --- .../seedu/address/model/person/PersonMatchesQueryPredicate.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index 10948c178d7..2369194851d 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -42,6 +42,8 @@ public boolean test(Person person) { //@@author rizkidelta private static boolean doesStringMatchQuery(String text, String query) { + requireNonNull(text); + // only need to process text, as query is already processed in the constructor text = processString(text); return text.contains(query); From 7ffa93f6298af960c1cf6923bc93551e43887a27 Mon Sep 17 00:00:00 2001 From: aureliony <39163684+aureliony@users.noreply.github.com> Date: Thu, 4 Apr 2024 14:30:42 +0800 Subject: [PATCH 13/13] Improve code based on PR comments --- src/main/java/seedu/address/logic/commands/FindCommand.java | 2 +- .../address/model/person/PersonMatchesQueryPredicate.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/FindCommand.java b/src/main/java/seedu/address/logic/commands/FindCommand.java index e8da4da328a..987b3172c72 100644 --- a/src/main/java/seedu/address/logic/commands/FindCommand.java +++ b/src/main/java/seedu/address/logic/commands/FindCommand.java @@ -11,7 +11,7 @@ import seedu.address.model.person.PersonMatchesQueryPredicate; /** - * Finds and lists all persons in the address book such that certain fields match the predicate, + * Finds and lists all persons in the address book such that certain fields match the predicate. * See {@code PersonMatchesQueryPredicate}. */ public class FindCommand extends Command { diff --git a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java index 2369194851d..af3ef82299a 100644 --- a/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java +++ b/src/main/java/seedu/address/model/person/PersonMatchesQueryPredicate.java @@ -11,8 +11,8 @@ */ public class PersonMatchesQueryPredicate implements Predicate { - private static final String whitespaceRegex = "\\s+"; - private static final String emptyString = ""; + private static final String STRIP_WHITESPACE_REGEX = "\\s+"; + private static final String EMPTY_STRING = ""; private final String query; @@ -29,7 +29,7 @@ public PersonMatchesQueryPredicate(String query) { } private static String processString(String str) { - return str.toLowerCase().replaceAll(whitespaceRegex, emptyString); + return str.toLowerCase().replaceAll(STRIP_WHITESPACE_REGEX, EMPTY_STRING); } //@@author rizkidelta