-
Notifications
You must be signed in to change notification settings - Fork 35
[W5][M11-3] Dannica Ong #39
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
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,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)} | ||
| */ | ||
|
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. 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(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| public class Person implements ReadOnlyPerson { | ||
|
|
||
| private Name name; | ||
| private Gender gender; | ||
| private Phone phone; | ||
| private Email email; | ||
| private Address address; | ||
|
|
@@ -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; | ||
|
|
@@ -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; } | ||
|
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. 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; | ||
|
|
@@ -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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
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. 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); | ||
| } | ||
| } | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
@@ -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); | ||
|
|
||
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.
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).