Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/seedu/addressbook/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()

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?

{
System.out.print("helloworld");
}


}
47 changes: 47 additions & 0 deletions src/seedu/addressbook/commands/FindByPhone.java
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

Choose a reason for hiding this comment

The 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 FindByPhoneCommand or FindByPhoneNumberCommand

{

Choose a reason for hiding this comment

The 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 ... extends Command {

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";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but won't this be printed as ... phone number isthe specified ...?


private final Set<String> keywords;

public FindByPhone(Set<String> keywords)
{

Choose a reason for hiding this comment

The 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;
}

}
4 changes: 4 additions & 0 deletions src/seedu/addressbook/data/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Choose a reason for hiding this comment

The 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 private and static

{
System.out.print("helloworld");
}
}
1 change: 1 addition & 0 deletions src/seedu/addressbook/data/person/Person.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public Address getAddress() {
public Set<Tag> getTags() {
return new HashSet<>(tags);
}


/**
* Replaces this person's tags with the tags in the argument tag set.
Expand Down
6 changes: 6 additions & 0 deletions src/seedu/addressbook/data/person/Phone.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down Expand Up @@ -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+"));

Choose a reason for hiding this comment

The 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() {
Expand Down