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][M11-3] Dannica Ong #39

Open
wants to merge 1 commit 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
14 changes: 5 additions & 9 deletions src/seedu/addressbook/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@
import java.util.Set;

import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList;
import seedu.addressbook.data.person.*;

Choose a reason for hiding this comment

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

Importing everything using "*" (also known as wildcard imports) is generally discouraged and it also violates coding style. It reduces interpretability as to what packages are imported. Please note that your IDE might tend to create this error. Find out how to avoid it (https://stackoverflow.com/questions/3348816/intellij-never-use-wildcard-imports).

import seedu.addressbook.data.tag.Tag;

/**
Expand All @@ -22,9 +16,9 @@ public class AddCommand extends Command {

public static final String MESSAGE_USAGE = COMMAND_WORD + ": Adds a person to the address book. "
+ "Contact details can be marked private by prepending 'p' to the prefix.\n"
+ "Parameters: NAME [p]p/PHONE [p]e/EMAIL [p]a/ADDRESS [t/TAG]...\n"
+ "Parameters: NAME g/GENDER [p]p/PHONE [p]e/EMAIL [p]a/ADDRESS [t/TAG]...\n"
+ "Example: " + COMMAND_WORD
+ " John Doe p/98765432 e/[email protected] a/311, Clementi Ave 2, #02-25 t/friends t/owesMoney";
+ " John Doe g/M p/98765432 e/[email protected] a/311, Clementi Ave 2, #02-25 t/friends t/owesMoney";

public static final String MESSAGE_SUCCESS = "New person added: %1$s";
public static final String MESSAGE_DUPLICATE_PERSON = "This person already exists in the address book";
Expand All @@ -37,6 +31,7 @@ public class AddCommand extends Command {
* @throws IllegalValueException if any of the raw values are invalid
*/
public AddCommand(String name,
String gender,
String phone, boolean isPhonePrivate,
String email, boolean isEmailPrivate,
String address, boolean isAddressPrivate,
Expand All @@ -47,6 +42,7 @@ public AddCommand(String name,
}
this.toAdd = new Person(
new Name(name),
new Gender(gender),
new Phone(phone, isPhonePrivate),
new Email(email, isEmailPrivate),
new Address(address, isAddressPrivate),
Expand Down
55 changes: 55 additions & 0 deletions src/seedu/addressbook/data/person/Gender.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package seedu.addressbook.data.person;

import seedu.addressbook.data.exception.IllegalValueException;

/**
* Represents a Person's gender in the address book.
* Guarantees: immutable; is valid as declared in {@link #isValidGender(String)}
*/

Choose a reason for hiding this comment

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

Good job for including class comments.

public class Gender {

public static final String EXAMPLE = "female";
public static final String MESSAGE_GENDER_CONSTRAINTS =
"Person's gender should be either female or male.";
public static final String GENDER_VALIDATION_REGEX = "[\\w+]";

public final String value;

/**
* Validates given gender.
*
* @throws IllegalValueException if given gender string is invalid.
*/
public Gender(String gender) throws IllegalValueException {
String trimmedGender = gender.trim();
if (!isValidGender(trimmedGender)) {
throw new IllegalValueException(MESSAGE_GENDER_CONSTRAINTS);
}
this.value = trimmedGender;
}

/**
* Returns true if the given string is a valid gender.
*/
public static boolean isValidGender(String test) {
return test.matches(GENDER_VALIDATION_REGEX);
}

@Override
public String toString() {
return value;
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof Gender // instanceof handles nulls
&& this.value.equals(((Gender) other).value)); // state check
}

@Override
public int hashCode() {
return value.hashCode();
}

}
11 changes: 8 additions & 3 deletions src/seedu/addressbook/data/person/Person.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
public class Person implements ReadOnlyPerson {

private Name name;
private Gender gender;
private Phone phone;
private Email email;
private Address address;
Expand All @@ -22,8 +23,9 @@ public class Person implements ReadOnlyPerson {
/**
* Assumption: Every field must be present and not null.
*/
public Person(Name name, Phone phone, Email email, Address address, Set<Tag> tags) {
public Person(Name name, Gender gender, Phone phone, Email email, Address address, Set<Tag> tags) {
this.name = name;
this.gender = gender;
this.phone = phone;
this.email = email;
this.address = address;
Expand All @@ -34,14 +36,17 @@ public Person(Name name, Phone phone, Email email, Address address, Set<Tag> tag
* Copy constructor.
*/
public Person(ReadOnlyPerson source) {
this(source.getName(), source.getPhone(), source.getEmail(), source.getAddress(), source.getTags());
this(source.getName(), source.getGender(), source.getPhone(), source.getEmail(), source.getAddress(), source.getTags());
}

@Override
public Name getName() {
return name;
}

@Override
public Gender getGender() { return gender; }

Choose a reason for hiding this comment

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

Probably would be better to stick to the K&R style of brackets as seen in the adjacent methods.


@Override
public Phone getPhone() {
return phone;
Expand Down Expand Up @@ -80,7 +85,7 @@ public boolean equals(Object other) {
@Override
public int hashCode() {
// use this method for custom fields hashing instead of implementing your own
return Objects.hash(name, phone, email, address, tags);
return Objects.hash(name, gender, phone, email, address, tags);
}

@Override
Expand Down
13 changes: 13 additions & 0 deletions src/seedu/addressbook/data/person/ReadOnlyPerson.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
public interface ReadOnlyPerson {

Name getName();
Gender getGender();
Phone getPhone();
Email getEmail();
Address getAddress();
Expand Down Expand Up @@ -39,6 +40,7 @@ default boolean hasSameData(ReadOnlyPerson other) {
return other == this // short circuit if same object
|| (other != null // this is first to avoid NPE below
&& other.getName().equals(this.getName()) // state checks here onwards
&& other.getGender().equals(this.getGender())
&& other.getPhone().equals(this.getPhone())
&& other.getEmail().equals(this.getEmail())
&& other.getAddress().equals(this.getAddress())
Expand All @@ -51,22 +53,30 @@ default boolean hasSameData(ReadOnlyPerson other) {
default String getAsTextShowAll() {
final StringBuilder builder = new StringBuilder();
final String detailIsPrivate = "(private) ";

builder.append(getName())

.append(" Gender: ");
builder.append(getGender())

.append(" Phone: ");
if (getPhone().isPrivate()) {
builder.append(detailIsPrivate);
}
builder.append(getPhone())

.append(" Email: ");
if (getEmail().isPrivate()) {
builder.append(detailIsPrivate);
}
builder.append(getEmail())

.append(" Address: ");
if (getAddress().isPrivate()) {
builder.append(detailIsPrivate);
}
builder.append(getAddress())

.append(" Tags: ");
for (Tag tag : getTags()) {
builder.append(tag);
Expand All @@ -80,6 +90,9 @@ default String getAsTextShowAll() {
default String getAsTextHidePrivate() {
final StringBuilder builder = new StringBuilder();
builder.append(getName());

builder.append(" Gender: ").append(getGender());

if (!getPhone().isPrivate()) {
builder.append(" Phone: ").append(getPhone());
}
Expand Down
3 changes: 3 additions & 0 deletions src/seedu/addressbook/parser/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public class Parser {

public static final Pattern PERSON_DATA_ARGS_FORMAT = // '/' forward slashes are reserved for delimiter prefixes
Pattern.compile("(?<name>[^/]+)"
+ " g/(?<gender>[^/]+)"
+ " (?<isPhonePrivate>p?)p/(?<phone>[^/]+)"
+ " (?<isEmailPrivate>p?)e/(?<email>[^/]+)"
+ " (?<isAddressPrivate>p?)a/(?<address>[^/]+)"
Expand Down Expand Up @@ -119,6 +120,8 @@ private Command prepareAdd(String args) {
return new AddCommand(
matcher.group("name"),

matcher.group("gender"),

matcher.group("phone"),
isPrivatePrefixPresent(matcher.group("isPhonePrivate")),

Expand Down
8 changes: 2 additions & 6 deletions src/seedu/addressbook/storage/AddressBookDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@

import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.UniquePersonList;
import seedu.addressbook.data.person.*;
import seedu.addressbook.data.tag.Tag;
import seedu.addressbook.storage.StorageFile.StorageOperationException;

Expand Down Expand Up @@ -55,6 +50,7 @@ private static Person decodePersonFromString(String encodedPerson)

return new Person(
new Name(matcher.group("name")),
new Gender(matcher.group("gender")),
new Phone(matcher.group("phone"), isPrivatePrefixPresent(matcher.group("isPhonePrivate"))),
new Email(matcher.group("email"), isPrivatePrefixPresent(matcher.group("isEmailPrivate"))),
new Address(matcher.group("address"), isPrivatePrefixPresent(matcher.group("isAddressPrivate"))),
Expand Down
2 changes: 2 additions & 0 deletions src/seedu/addressbook/storage/AddressBookEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ private static String encodePersonToString(Person person) {

encodedPersonBuilder.append(person.getName());

encodedPersonBuilder.append("g/").append(person.getGender().value);

encodedPersonBuilder.append(person.getPhone().isPrivate() ? " p" : " ");
encodedPersonBuilder.append("p/").append(person.getPhone().value);

Expand Down
29 changes: 20 additions & 9 deletions test/java/seedu/addressbook/commands/AddCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Gender;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
Expand All @@ -32,16 +33,25 @@ public class AddCommandTest {
public void addCommand_invalidName_throwsException() {
final String[] invalidNames = { "", " ", "[]\\[;]" };
for (String name : invalidNames) {
assertConstructingInvalidAddCmdThrowsException(name, Phone.EXAMPLE, true, Email.EXAMPLE, false,
assertConstructingInvalidAddCmdThrowsException(name, Gender.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE, false,
Address.EXAMPLE, true, EMPTY_STRING_SET);
}
}

@Test
public void addCommand_invalidGender_throwsException() {
final String[] invalidGenders = { "", " ", "F/M", "[]\\[;]", "f", "m", "JSG" };
for (String gender : invalidGenders) {
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, gender, Phone.EXAMPLE, true, Email.EXAMPLE, true,
Address.EXAMPLE, false, EMPTY_STRING_SET);
}
}

Choose a reason for hiding this comment

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

Good job on working with JUnit tests!


@Test
public void addCommand_invalidPhone_throwsException() {
final String[] invalidNumbers = { "", " ", "1234-5678", "[]\\[;]", "abc", "a123", "+651234" };
for (String number : invalidNumbers) {
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, number, false, Email.EXAMPLE, true,
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Gender.EXAMPLE, number, false, Email.EXAMPLE, true,
Address.EXAMPLE, false, EMPTY_STRING_SET);
}
}
Expand All @@ -51,7 +61,7 @@ public void addCommand_invalidEmail_throwsException() {
final String[] invalidEmails = { "", " ", "def.com", "@", "@def", "@def.com", "abc@",
"@invalid@email", "invalid@email!", "!invalid@email" };
for (String email : invalidEmails) {
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, false, email, false,
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Gender.EXAMPLE, Phone.EXAMPLE, false, email, false,
Address.EXAMPLE, false, EMPTY_STRING_SET);
}
}
Expand All @@ -60,7 +70,7 @@ public void addCommand_invalidEmail_throwsException() {
public void addCommand_invalidAddress_throwsException() {
final String[] invalidAddresses = { "", " " };
for (String address : invalidAddresses) {
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Gender.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
true, address, true, EMPTY_STRING_SET);
}
}
Expand All @@ -71,7 +81,7 @@ public void addCommand_invalidTags_throwsException() {
{ "", " " } };
for (String[] tags : invalidTags) {
Set<String> tagsToAdd = new HashSet<>(Arrays.asList(tags));
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
assertConstructingInvalidAddCmdThrowsException(Name.EXAMPLE, Gender.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
true, Address.EXAMPLE, false, tagsToAdd);
}
}
Expand All @@ -80,30 +90,31 @@ public void addCommand_invalidTags_throwsException() {
* Asserts that attempting to construct an add command with the supplied
* invalid data throws an IllegalValueException
*/
private void assertConstructingInvalidAddCmdThrowsException(String name, String phone,
private void assertConstructingInvalidAddCmdThrowsException(String name, String gender, String phone,
boolean isPhonePrivate, String email, boolean isEmailPrivate, String address,
boolean isAddressPrivate, Set<String> tags) {
try {
new AddCommand(name, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate,
new AddCommand(name, gender, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate,
tags);
} catch (IllegalValueException e) {
return;
}
String error = String.format(
"An add command was successfully constructed with invalid input: %s %s %s %s %s %s %s %s",
name, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate, tags);
name, gender, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate, tags);
fail(error);
}

@Test
public void addCommand_validData_correctlyConstructed() throws Exception {
AddCommand command = new AddCommand(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE, false,
AddCommand command = new AddCommand(Name.EXAMPLE, Gender.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE, false,
Address.EXAMPLE, true, EMPTY_STRING_SET);
ReadOnlyPerson p = command.getPerson();

// TODO: add comparison of tags to person.equals and equality methods to
// individual fields that compare privacy to simplify this
assertEquals(Name.EXAMPLE, p.getName().fullName);
assertEquals(Gender.EXAMPLE, p.getGender().value);
assertEquals(Phone.EXAMPLE, p.getPhone().value);
assertTrue(p.getPhone().isPrivate());
assertEquals(Email.EXAMPLE, p.getEmail().value);
Expand Down
17 changes: 6 additions & 11 deletions test/java/seedu/addressbook/commands/DeleteCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@
import seedu.addressbook.common.Messages;
import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.*;
import seedu.addressbook.data.person.UniquePersonList.PersonNotFoundException;
import seedu.addressbook.ui.TextUi;
import seedu.addressbook.util.TestUtil;
Expand All @@ -32,13 +27,13 @@ public class DeleteCommandTest {

@Before
public void setUp() throws Exception {
Person johnDoe = new Person(new Name("John Doe"), new Phone("61234567", false),
Person johnDoe = new Person(new Name("John Doe"), new Gender("M"), new Phone("61234567", false),
new Email("[email protected]", false), new Address("395C Ben Road", false), Collections.emptySet());
Person janeDoe = new Person(new Name("Jane Doe"), new Phone("91234567", false),
Person janeDoe = new Person(new Name("Jane Doe"), new Gender("F"), new Phone("91234567", false),
new Email("[email protected]", false), new Address("33G Ohm Road", false), Collections.emptySet());
Person samDoe = new Person(new Name("Sam Doe"), new Phone("63345566", false),
Person samDoe = new Person(new Name("Sam Doe"), new Gender("M"), new Phone("63345566", false),
new Email("[email protected]", false), new Address("55G Abc Road", false), Collections.emptySet());
Person davidGrant = new Person(new Name("David Grant"), new Phone("61121122", false),
Person davidGrant = new Person(new Name("David Grant"), new Gender("M"), new Phone("61121122", false),
new Email("[email protected]", false), new Address("44H Define Road", false),
Collections.emptySet());

Expand All @@ -64,7 +59,7 @@ public void execute_noPersonDisplayed_returnsInvalidIndexMessage() {
@Test
public void execute_targetPersonNotInAddressBook_returnsPersonNotFoundMessage()
throws IllegalValueException {
Person notInAddressBookPerson = new Person(new Name("Not In Book"), new Phone("63331444", false),
Person notInAddressBookPerson = new Person(new Name("Not In Book"), new Gender("M"), new Phone("63331444", false),
new Email("[email protected]", false), new Address("156D Grant Road", false), Collections.emptySet());
List<ReadOnlyPerson> listWithPersonNotInAddressBook = TestUtil.createList(notInAddressBookPerson);

Expand Down
Loading