[W5][W16-3]Zhang Jiayu #146
Conversation
|
It's really a good job to find a person by some other details. |
| 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.
Variable name does not suit the current operation, since you are retrieving the phone number of a person, not their name (hence wordsInName).
| @@ -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.
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:
https://www.vogella.com/tutorials/JavaRegularExpressions/article.html
| 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.
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.
| @@ -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.
Documentation for this method is missing.
|
|
||
| /** | ||
| * Finds and lists all persons in address book whose telephone number is the argument keywords. | ||
| * Keyword matching is case sensitive. |
There was a problem hiding this comment.
This comment here does not make sense, since case sensitivity are for characters, not numbers.
Update the User Guide; Add a new command findPhoneNumber to find a person's information through input his telephone number.