-
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-03]Fan Dongzhe #110
base: master
Are you sure you want to change the base?
Changes from all commits
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,47 @@ | ||
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; | ||
|
||
public class FindByPhone extends Command | ||
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. Follow the naming convention in the other Command files, so this class should be something like |
||
{ | ||
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 review the coding convention. This curly brace should appear at the end of the previous line |
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, but won't this be printed as |
||
|
||
private final Set<String> keywords; | ||
|
||
public FindByPhone(Set<String> keywords) | ||
{ | ||
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. Curly brace on previous line, as above |
||
this.keywords = keywords; | ||
} | ||
|
||
public Set<String> getKeywords() | ||
{ | ||
return new HashSet<>(keywords); | ||
} | ||
|
||
public CommandResult execute() | ||
{ | ||
final List<ReadOnlyPerson> personsFound = getPersonsWithNameContainingAnyKeyword(keywords); | ||
return new CommandResult(getMessageForPersonListShownSummary(personsFound), personsFound); | ||
} | ||
|
||
private List<ReadOnlyPerson> getPersonsWithNameContainingAnyKeyword(Set<String> keywords) | ||
{ | ||
final List<ReadOnlyPerson> matchedPersons = new ArrayList<>(); | ||
for (ReadOnlyPerson person : addressBook.getAllPersons()) { | ||
final Set<String> PhoneNumber = new HashSet<>(person.getPhone().getPhone()); | ||
if (!Collections.disjoint(PhoneNumber, keywords)) { | ||
matchedPersons.add(person); | ||
} | ||
} | ||
return matchedPersons; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please leave a newline between functions. Also, there's a double space between your |
||
{ | ||
System.out.print("helloworld"); | ||
} | ||
} |
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)} | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
} | ||
|
||
@Override | ||
public String toString() { | ||
|
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 shouldn't be here, should it?