Skip to content

Commit

Permalink
Merge pull request nus-cs2103-AY2021S1#107 from jerrylchong/branch-Fi…
Browse files Browse the repository at this point in the history
…nd-Fix

Fix FindCommand issues and tests
  • Loading branch information
royleochan authored Oct 11, 2020
2 parents fe674a9 + 883ed01 commit 3b348b4
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,12 +38,18 @@ public FindCommand parse(String args) throws ParseException {
if (arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_TAG)) {
Set<String> nameSet = ParserUtil.parseAllNames(argMultimap.getAllValues(PREFIX_NAME));
Set<Tag> 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<String> 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<Tag> tagSet = parseTagsForFind(argMultimap.getAllValues(PREFIX_TAG)).orElse(new HashSet<>());
return new FindCommand(
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ public static Name parseName(String name) throws ParseException {
*/
public static Set<String> parseAllNames(Collection<String> names) throws ParseException {
requireNonNull(names);
final Set<String> nameSet = new HashSet<>();
for (String name : names) {
nameSet.add(name);
}
Set<String> nameSet = new HashSet<>(names);
return nameSet;
}

Expand Down
91 changes: 74 additions & 17 deletions src/test/java/seedu/address/logic/parser/FindCommandParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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<String> nameList = new ArrayList<>();
// nameList.add("Alice");
// nameList.add("Bob");
// nameList.add("Candy");
// ArrayList<Tag> 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<String> nameSet = new HashSet<>();
nameSet.add("Alice");
nameSet.add("Bob");
nameSet.add("Candy");
ArrayList<String> nameList = new ArrayList<>(nameSet);
Set<Tag> tagSet = new HashSet<>();
tagSet.add(new Tag("tag"));
tagSet.add(new Tag("person"));
ArrayList<Tag> 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<String> nameList = new ArrayList<>();
nameList.add("Alice");
ArrayList<Tag> 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<String> nameSet = new HashSet<>();
nameSet.add("Alice");
nameSet.add("Bob");
nameSet.add("Candy");
ArrayList<String> 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<Tag> 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<Tag> tagSet = new HashSet<>();
tagSet.add(new Tag("tag"));
tagSet.add(new Tag("person"));
ArrayList<Tag> tagList = new ArrayList<>(tagSet);
FindCommand expectedFindCommand =
new FindCommand(new PersonHasTagsPredicate(tagList));
assertParseSuccess(parser, " t/tag t/person ", expectedFindCommand);
}
}

0 comments on commit 3b348b4

Please sign in to comment.