[W5][W16-03]Fan Dongzhe #110
Conversation
|
new command |
|
Yes, telephone number is unique. You can find the exact person through their phone number. |
|
|
||
| import seedu.addressbook.data.person.ReadOnlyPerson; | ||
|
|
||
| public class FindByPhone extends Command |
There was a problem hiding this comment.
Follow the naming convention in the other Command files, so this class should be something like FindByPhoneCommand or FindByPhoneNumberCommand
| import seedu.addressbook.data.person.ReadOnlyPerson; | ||
|
|
||
| public class FindByPhone extends Command | ||
| { |
There was a problem hiding this comment.
Please review the coding convention. This curly brace should appear at the end of the previous line ... extends Command {
| private final Set<String> keywords; | ||
|
|
||
| public FindByPhone(Set<String> keywords) | ||
| { |
There was a problem hiding this comment.
Curly brace on previous line, as above
| { | ||
| public static final String COMMAND_WORD = "find by phone number"; | ||
|
|
||
| public static final String MESSAGE_USAGE = COMMAND_WORD + "Finds all persons whose phone number is" + "the specified keywords (case-sensitive) and displays them as a list with index numbers.\n"+ "Parameters: KEYWORD [MORE_KEYWORDS]...\n"+ "Example" + COMMAND_WORD + "alice"; |
There was a problem hiding this comment.
Nit, but won't this be printed as ... phone number isthe specified ...?
| @@ -35,6 +38,9 @@ public Phone(String phone, boolean isPrivate) throws IllegalValueException { | |||
| public static boolean isValidPhone(String test) { | |||
| return test.matches(PHONE_VALIDATION_REGEX); | |||
| } | |||
| public List<String> getPhone() { | |||
| return Arrays.asList(value.split("\\s+")); | |||
There was a problem hiding this comment.
According to the regex, the phone number can only be a string of digits. Then, there is no need to split on whitespace (since there will be none). Also, is it necessary to return the results in a list? Since there will only be 1 string.
| @@ -74,4 +74,8 @@ public boolean equals(Object other) { | |||
| || (other instanceof AddressBook // instanceof handles nulls | |||
| && this.allPersons.equals(((AddressBook) other).allPersons)); | |||
| } | |||
| private static void helloworld() | |||
There was a problem hiding this comment.
Please leave a newline between functions. Also, there's a double space between your private and static
| @@ -126,6 +126,10 @@ private StorageFile initializeStorage(String[] launchArgs) throws InvalidStorage | |||
| boolean isStorageFileSpecifiedByUser = launchArgs.length > 0; | |||
| return isStorageFileSpecifiedByUser ? new StorageFile(launchArgs[0]) : new StorageFile(); | |||
| } | |||
| private static void helloworld() | |||
It's a simple test