From 9f62ff52622df498ebe0be2316a6dc7ec6b4dcb4 Mon Sep 17 00:00:00 2001 From: Jerryl Chong Date: Sun, 11 Oct 2020 12:21:04 +0800 Subject: [PATCH 1/2] Modify FindCommandParser parse logic --- .../seedu/address/logic/parser/FindCommandParser.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/seedu/address/logic/parser/FindCommandParser.java b/src/main/java/seedu/address/logic/parser/FindCommandParser.java index 707d4771c0d..2ae7366fbd0 100644 --- a/src/main/java/seedu/address/logic/parser/FindCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/FindCommandParser.java @@ -15,6 +15,7 @@ import seedu.address.logic.commands.FindCommand; import seedu.address.logic.parser.exceptions.ParseException; +import seedu.address.model.person.NameContainsKeywordsPredicate; import seedu.address.model.person.PersonHasTagsAndNamePredicate; import seedu.address.model.person.PersonHasTagsPredicate; import seedu.address.model.tag.Tag; @@ -37,12 +38,18 @@ public FindCommand parse(String args) throws ParseException { if (arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_TAG)) { Set nameSet = ParserUtil.parseAllNames(argMultimap.getAllValues(PREFIX_NAME)); Set tagSet = parseTagsForFind(argMultimap.getAllValues(PREFIX_TAG)).orElse(new HashSet<>()); + if ((nameSet.size() == 1 && nameSet.contains("")) || tagSet.isEmpty()) { + throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + } return new FindCommand( new PersonHasTagsAndNamePredicate(new ArrayList<>(nameSet), new ArrayList<>(tagSet))); } else if (arePrefixesPresent(argMultimap, PREFIX_NAME)) { Set nameSet = ParserUtil.parseAllNames(argMultimap.getAllValues(PREFIX_NAME)); + if (nameSet.size() == 1 && nameSet.contains("")) { + throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + } return new FindCommand( - new PersonHasTagsAndNamePredicate(new ArrayList<>(nameSet), new ArrayList<>())); + new NameContainsKeywordsPredicate(new ArrayList<>(nameSet))); } else if (arePrefixesPresent(argMultimap, PREFIX_TAG)) { Set tagSet = parseTagsForFind(argMultimap.getAllValues(PREFIX_TAG)).orElse(new HashSet<>()); return new FindCommand( From 883ed01b0b5619f6c4100387491f4fc0bad5931b Mon Sep 17 00:00:00 2001 From: Jerryl Chong Date: Sun, 11 Oct 2020 12:41:44 +0800 Subject: [PATCH 2/2] Fix and add tests for FindCommandParser --- .../address/logic/parser/ParserUtil.java | 5 +- .../logic/parser/FindCommandParserTest.java | 91 +++++++++++++++---- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/src/main/java/seedu/address/logic/parser/ParserUtil.java b/src/main/java/seedu/address/logic/parser/ParserUtil.java index a72b973a13a..4a75348b0ad 100644 --- a/src/main/java/seedu/address/logic/parser/ParserUtil.java +++ b/src/main/java/seedu/address/logic/parser/ParserUtil.java @@ -57,10 +57,7 @@ public static Name parseName(String name) throws ParseException { */ public static Set parseAllNames(Collection names) throws ParseException { requireNonNull(names); - final Set nameSet = new HashSet<>(); - for (String name : names) { - nameSet.add(name); - } + Set nameSet = new HashSet<>(names); return nameSet; } diff --git a/src/test/java/seedu/address/logic/parser/FindCommandParserTest.java b/src/test/java/seedu/address/logic/parser/FindCommandParserTest.java index 725102d7b12..1ff7dcf2af1 100644 --- a/src/test/java/seedu/address/logic/parser/FindCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/FindCommandParserTest.java @@ -5,11 +5,15 @@ import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; import org.junit.jupiter.api.Test; import seedu.address.logic.commands.FindCommand; +import seedu.address.model.person.NameContainsKeywordsPredicate; import seedu.address.model.person.PersonHasTagsAndNamePredicate; +import seedu.address.model.person.PersonHasTagsPredicate; import seedu.address.model.tag.Tag; public class FindCommandParserTest { @@ -18,7 +22,24 @@ public class FindCommandParserTest { @Test public void parse_emptyArg_throwsParseException() { - assertParseFailure(parser, " ", String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // no arguments + assertParseFailure(parser, " ", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // name prefix but no name argument + assertParseFailure(parser, " n/", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // tag prefix but no tag argument + assertParseFailure(parser, "t/", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // name and tag prefix but no name argument + assertParseFailure(parser, " n/ t/tag", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // name and tag prefix but no tag argument + assertParseFailure(parser, " n/name t/", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); + // name and tag prefix but no arguments + assertParseFailure(parser, " n/ t/", + String.format(MESSAGE_INVALID_COMMAND_FORMAT, FindCommand.MESSAGE_USAGE)); } @Test @@ -36,29 +57,65 @@ public void parse_validArgs_returnsFindCommand() { assertParseSuccess(parser, " \n n/Alice \n t/tag\t", expectedFindCommand); } - //TODO: Fix this test case - // @Test - // public void parse_multipleNames_returnsFindCommand() { - // // no leading and trailing whitespaces - // ArrayList nameList = new ArrayList<>(); - // nameList.add("Alice"); - // nameList.add("Bob"); - // nameList.add("Candy"); - // ArrayList tagList = new ArrayList<>(); - // tagList.add(new Tag("tag")); - // FindCommand expectedFindCommand = - // new FindCommand(new PersonHasTagsAndNamePredicate(nameList, tagList)); - // assertParseSuccess(parser, " n/Alice n/Bob n/Candy t/tag", expectedFindCommand); - // } + @Test + public void parse_multipleNamesAndMultipleTags_returnsFindCommand() { + // no leading and trailing whitespaces + Set nameSet = new HashSet<>(); + nameSet.add("Alice"); + nameSet.add("Bob"); + nameSet.add("Candy"); + ArrayList nameList = new ArrayList<>(nameSet); + Set tagSet = new HashSet<>(); + tagSet.add(new Tag("tag")); + tagSet.add(new Tag("person")); + ArrayList tagList = new ArrayList<>(tagSet); + FindCommand expectedFindCommand = + new FindCommand(new PersonHasTagsAndNamePredicate(nameList, tagList)); + assertParseSuccess(parser, " n/Alice n/Bob n/Candy t/tag t/person", expectedFindCommand); + } @Test public void parse_singleNameAndNoTag_returnsFindCommand() { // no leading and trailing whitespaces ArrayList nameList = new ArrayList<>(); nameList.add("Alice"); - ArrayList tagList = new ArrayList<>(); FindCommand expectedFindCommand = - new FindCommand(new PersonHasTagsAndNamePredicate(nameList, tagList)); + new FindCommand(new NameContainsKeywordsPredicate(nameList)); assertParseSuccess(parser, " n/Alice ", expectedFindCommand); } + + @Test + public void parse_multipleNamesAndNoTag_returnsFindCommand() { + // no leading and trailing whitespaces + Set nameSet = new HashSet<>(); + nameSet.add("Alice"); + nameSet.add("Bob"); + nameSet.add("Candy"); + ArrayList nameList = new ArrayList<>(nameSet); + FindCommand expectedFindCommand = + new FindCommand(new NameContainsKeywordsPredicate(nameList)); + assertParseSuccess(parser, " n/Alice n/Bob n/Candy", expectedFindCommand); + } + + @Test + public void parse_noNameAndSingleTag_returnsFindCommand() { + // no leading and trailing whitespaces + ArrayList tagList = new ArrayList<>(); + tagList.add(new Tag("tag")); + FindCommand expectedFindCommand = + new FindCommand(new PersonHasTagsPredicate(tagList)); + assertParseSuccess(parser, " t/tag ", expectedFindCommand); + } + + @Test + public void parse_noNameAndMultipleTags_returnsFindCommand() { + // no leading and trailing whitespaces + Set tagSet = new HashSet<>(); + tagSet.add(new Tag("tag")); + tagSet.add(new Tag("person")); + ArrayList tagList = new ArrayList<>(tagSet); + FindCommand expectedFindCommand = + new FindCommand(new PersonHasTagsPredicate(tagList)); + assertParseSuccess(parser, " t/tag t/person ", expectedFindCommand); + } }