-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[W5][W16-3]Zhang Jiayu #146
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package seedu.addressbook.commands; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Set; | ||
|
||
import seedu.addressbook.data.person.ReadOnlyPerson; | ||
|
||
/** | ||
* Finds and lists all persons in address book whose telephone number is the argument keywords. | ||
* Keyword matching is case sensitive. | ||
*/ | ||
public class FindPhoneNumberCommand extends Command { | ||
|
||
public static final String COMMAND_WORD = "findPhoneNumber"; | ||
|
||
public static final String MESSAGE_USAGE = COMMAND_WORD + ": Finds the person whose telephone number is " | ||
+ "the specified keywords (case-sensitive) and displays the person's information.\n" | ||
+ "Parameters: KEYWORD [MORE_KEYWORDS]...\n" | ||
+ "Example: " + COMMAND_WORD + " 12345678"; | ||
|
||
private final Set<String> keywords; | ||
|
||
public FindPhoneNumberCommand(Set<String> keywords) { | ||
this.keywords = keywords; | ||
} | ||
|
||
/** | ||
* Returns a copy of keywords in this command. | ||
*/ | ||
public Set<String> getKeywords() { | ||
return new HashSet<>(keywords); | ||
} | ||
|
||
@Override | ||
public CommandResult execute() { | ||
final List<ReadOnlyPerson> numberFound = getPersonsWithNameContainingAnyKeyword(keywords); | ||
return new CommandResult(getMessageForPersonListShownSummary(numberFound), numberFound); | ||
} | ||
|
||
/** | ||
* Retrieves all persons in the address book whose names contain some of the specified keywords. | ||
* | ||
* @param keywords for searching | ||
* @return list of persons found | ||
*/ | ||
private List<ReadOnlyPerson> getPersonsWithNameContainingAnyKeyword(Set<String> keywords) { | ||
final List<ReadOnlyPerson> matchedPersons = new ArrayList<>(); | ||
for (ReadOnlyPerson person : addressBook.getAllPersons()) { | ||
final Set<String> wordsInName = new HashSet<>(person.getPhone().getWordsInPhone()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Variable name does not suit the current operation, since you are retrieving the phone number of a person, not their name (hence wordsInName). |
||
if (!Collections.disjoint(wordsInName, keywords)) { | ||
matchedPersons.add(person); | ||
} | ||
} | ||
return matchedPersons; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ | |
|
||
import seedu.addressbook.data.exception.IllegalValueException; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
/** | ||
* Represents a Person's phone number in the address book. | ||
* Guarantees: immutable; is valid as declared in {@link #isValidPhone(String)} | ||
|
@@ -36,6 +39,10 @@ public static boolean isValidPhone(String test) { | |
return test.matches(PHONE_VALIDATION_REGEX); | ||
} | ||
|
||
public List<String> getWordsInPhone() { | ||
return Arrays.asList(value.split("\\s+")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you look at the regex for the phone validation (line 16), the validation will only allow 1 or more digits. Thus, "1234 5678" would have been rejected as a phone number. So, there is no need to split the value by spaces. It is also recommended for you to change the method name, since there shouldn't be "words" in a phone number. If you need help in understanding regex, below is an article that may help you: |
||
} | ||
|
||
@Override | ||
public String toString() { | ||
return value; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,17 +11,7 @@ | |
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
import seedu.addressbook.commands.AddCommand; | ||
import seedu.addressbook.commands.ClearCommand; | ||
import seedu.addressbook.commands.Command; | ||
import seedu.addressbook.commands.DeleteCommand; | ||
import seedu.addressbook.commands.ExitCommand; | ||
import seedu.addressbook.commands.FindCommand; | ||
import seedu.addressbook.commands.HelpCommand; | ||
import seedu.addressbook.commands.IncorrectCommand; | ||
import seedu.addressbook.commands.ListCommand; | ||
import seedu.addressbook.commands.ViewAllCommand; | ||
import seedu.addressbook.commands.ViewCommand; | ||
import seedu.addressbook.commands.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please follow the Java coding standard on the website. You are expected to list all your imported class explicitly. If your IDE is automatically converting this, please do change the settings to prevent it from doing so. |
||
import seedu.addressbook.data.exception.IllegalValueException; | ||
|
||
/** | ||
|
@@ -88,6 +78,9 @@ public Command parseCommand(String userInput) { | |
case ListCommand.COMMAND_WORD: | ||
return new ListCommand(); | ||
|
||
case FindPhoneNumberCommand.COMMAND_WORD: | ||
return prepareFindPhoneNumber(arguments); | ||
|
||
case ViewCommand.COMMAND_WORD: | ||
return prepareView(arguments); | ||
|
||
|
@@ -248,5 +241,18 @@ private Command prepareFind(String args) { | |
return new FindCommand(keywordSet); | ||
} | ||
|
||
private Command prepareFindPhoneNumber(String args) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation for this method is missing. |
||
final Matcher matcher = KEYWORDS_ARGS_FORMAT.matcher(args.trim()); | ||
if (!matcher.matches()) { | ||
return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, | ||
FindCommand.MESSAGE_USAGE)); | ||
} | ||
|
||
// keywords delimited by whitespace | ||
final String[] keywords = matcher.group("keywords").split("\\s+"); | ||
final Set<String> keywordSet = new HashSet<>(Arrays.asList(keywords)); | ||
return new FindPhoneNumberCommand(keywordSet); | ||
} | ||
|
||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment here does not make sense, since case sensitivity are for characters, not numbers.