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

Refactor models: privatize attributes and methods in Person fields #32

Merged
merged 7 commits into from
Mar 3, 2024
Merged
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
31 changes: 17 additions & 14 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import seedu.address.model.person.fields.Name;
import seedu.address.model.person.fields.Phone;
import seedu.address.model.person.fields.Tags;
import seedu.address.model.tag.Tag;

/**
* Contains utility methods used for parsing strings in the various *Parser classes.
Expand Down Expand Up @@ -43,10 +42,11 @@ public static Index parseIndex(String oneBasedIndex) throws ParseException {
public static Name parseName(String name) throws ParseException {
requireNonNull(name);
String trimmedName = name.trim();
if (!Name.isValidName(trimmedName)) {
throw new ParseException(Name.MESSAGE_CONSTRAINTS);
try {
return new Name(trimmedName);
} catch (IllegalArgumentException e) {
throw new ParseException(e.getMessage());
}
return new Name(trimmedName);
}

/**
Expand All @@ -58,10 +58,11 @@ public static Name parseName(String name) throws ParseException {
public static Phone parsePhone(String phone) throws ParseException {
requireNonNull(phone);
String trimmedPhone = phone.trim();
if (!Phone.isValidPhone(trimmedPhone)) {
throw new ParseException(Phone.MESSAGE_CONSTRAINTS);
try {
return new Phone(trimmedPhone);
} catch (IllegalArgumentException e) {
throw new ParseException(e.getMessage());
}
return new Phone(trimmedPhone);
}

/**
Expand All @@ -73,10 +74,11 @@ public static Phone parsePhone(String phone) throws ParseException {
public static Address parseAddress(String address) throws ParseException {
requireNonNull(address);
String trimmedAddress = address.trim();
if (!Address.isValidAddress(trimmedAddress)) {
throw new ParseException(Address.MESSAGE_CONSTRAINTS);
try {
return new Address(trimmedAddress);
} catch (IllegalArgumentException e) {
throw new ParseException(e.getMessage());
}
return new Address(trimmedAddress);
}

/**
Expand All @@ -88,10 +90,11 @@ public static Address parseAddress(String address) throws ParseException {
public static Email parseEmail(String email) throws ParseException {
requireNonNull(email);
String trimmedEmail = email.trim();
if (!Email.isValidEmail(trimmedEmail)) {
throw new ParseException(Email.MESSAGE_CONSTRAINTS);
try {
return new Email(trimmedEmail);
} catch (IllegalArgumentException e) {
throw new ParseException(e.getMessage());
}
return new Email(trimmedEmail);
}

/**
Expand All @@ -102,7 +105,7 @@ public static Tags parseTags(Collection<String> tags) throws ParseException {
try {
return new Tags(tags.toArray(new String[0]));
} catch (IllegalArgumentException e) {
throw new ParseException(Tag.MESSAGE_CONSTRAINTS);
throw new ParseException(e.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public NameContainsKeywordsPredicate(List<String> keywords) {
@Override
public boolean test(Person person) {
return keywords.stream()
.anyMatch(keyword -> StringUtil.containsWordIgnoreCase(person.getName().name, keyword));
.anyMatch(keyword -> StringUtil.containsWordIgnoreCase(person.getName().toString(), keyword));
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/seedu/address/model/person/fields/Address.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
*/
public class Address implements Field {

public static final String MESSAGE_CONSTRAINTS = "Addresses can take any values, and it should not be blank";
private static final String MESSAGE_CONSTRAINTS = "Addresses can take any values, and it should not be blank";

/*
* The first character of the address must not be a whitespace,
* otherwise " " (a blank string) becomes a valid input.
*/
public static final String VALIDATION_REGEX = "[^\\s].*";
private static final String VALIDATION_REGEX = "[^\\s].*";

public final String address;
private final String address;

/**
* Constructs an {@code Address}.
Expand All @@ -28,14 +28,14 @@ public class Address implements Field {
*/
public Address(String address) {
requireNonNull(address);
checkArgument(isValidAddress(address), MESSAGE_CONSTRAINTS);
checkArgument(isValid(address), MESSAGE_CONSTRAINTS);
this.address = address;
}

/**
* Returns true if a given string is a valid email.
*/
public static boolean isValidAddress(String test) {
private static boolean isValid(String test) {
return test.matches(VALIDATION_REGEX);
}

Expand Down
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/model/person/fields/Email.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
public class Email implements Field {

private static final String SPECIAL_CHARACTERS = "+_.-";
public static final String MESSAGE_CONSTRAINTS = "Emails should be of the format local-part@domain "
private static final String MESSAGE_CONSTRAINTS = "Emails should be of the format local-part@domain "
+ "and adhere to the following constraints:\n"
+ "1. The local-part should only contain alphanumeric characters and these special characters, excluding "
+ "the parentheses, (" + SPECIAL_CHARACTERS + "). The local-part may not start or end with any special "
Expand All @@ -33,7 +33,7 @@ public class Email implements Field {
private static final String DOMAIN_REGEX = "(" + DOMAIN_PART_REGEX + "\\.)*" + DOMAIN_LAST_PART_REGEX;
public static final String VALIDATION_REGEX = LOCAL_PART_REGEX + "@" + DOMAIN_REGEX;

public final String email;
private final String email;

/**
* Constructs an {@code Email}.
Expand All @@ -42,14 +42,14 @@ public class Email implements Field {
*/
public Email(String email) {
requireNonNull(email);
checkArgument(isValidEmail(email), MESSAGE_CONSTRAINTS);
checkArgument(isValid(email), MESSAGE_CONSTRAINTS);
this.email = email;
}

/**
* Returns if a given string is a valid email.
*/
public static boolean isValidEmail(String test) {
private static boolean isValid(String test) {
return test.matches(VALIDATION_REGEX);
}

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/seedu/address/model/person/fields/Name.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@
*/
public class Name implements Field {

public static final String MESSAGE_CONSTRAINTS =
private static final String MESSAGE_CONSTRAINTS =
"Names should only contain alphanumeric characters and spaces, and it should not be blank";

/*
* The first character of the address must not be a whitespace,
* otherwise " " (a blank string) becomes a valid input.
*/
public static final String VALIDATION_REGEX = "[\\p{Alnum}][\\p{Alnum} ]*";
private static final String VALIDATION_REGEX = "[\\p{Alnum}][\\p{Alnum} ]*";

public final String name;
private final String name;

/**
* Constructs a {@code Name}.
Expand All @@ -29,14 +29,14 @@ public class Name implements Field {
*/
public Name(String name) {
requireNonNull(name);
checkArgument(isValidName(name), MESSAGE_CONSTRAINTS);
checkArgument(isValid(name), MESSAGE_CONSTRAINTS);
this.name = name;
}

/**
* Returns true if a given string is a valid name.
*/
public static boolean isValidName(String test) {
private static boolean isValid(String test) {
return test.matches(VALIDATION_REGEX);
}

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/seedu/address/model/person/fields/Phone.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
*/
public class Phone implements Field {

public static final String MESSAGE_CONSTRAINTS =
private static final String MESSAGE_CONSTRAINTS =
"Phone numbers should only contain numbers, and it should be at least 3 digits long";
public static final String VALIDATION_REGEX = "\\d{3,}";
public final String phone;
private static final String VALIDATION_REGEX = "\\d{3,}";
private final String phone;

/**
* Constructs a {@code Phone}.
Expand All @@ -23,14 +23,14 @@ public class Phone implements Field {
*/
public Phone(String phone) {
requireNonNull(phone);
checkArgument(isValidPhone(phone), MESSAGE_CONSTRAINTS);
checkArgument(isValid(phone), MESSAGE_CONSTRAINTS);
this.phone = phone;
}

/**
* Returns true if a given string is a valid phone number.
*/
public static boolean isValidPhone(String test) {
private static boolean isValid(String test) {
return test.matches(VALIDATION_REGEX);
}

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/seedu/address/model/tag/Tag.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
*/
public class Tag {

public static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric";
public static final String VALIDATION_REGEX = "\\p{Alnum}+";
private static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric";
private static final String VALIDATION_REGEX = "\\p{Alnum}+";

public final String tagName;
private final String tagName;

/**
* Constructs a {@code Tag}.
Expand All @@ -24,7 +24,7 @@ public class Tag {
public Tag(String tagName) {
requireNonNull(tagName);
tagName = tagName.trim();
checkArgument(isValidTagName(tagName), MESSAGE_CONSTRAINTS);
checkArgument(isValid(tagName), MESSAGE_CONSTRAINTS);
this.tagName = tagName;
}

Expand All @@ -36,7 +36,7 @@ public String get() {
/**
* Returns true if a given string is a valid tag name.
*/
public static boolean isValidTagName(String test) {
private static boolean isValid(String test) {
return test.matches(VALIDATION_REGEX);
}

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/seedu/address/ui/PersonCard.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import javafx.scene.layout.HBox;
import javafx.scene.layout.Region;
import seedu.address.model.person.Person;
import seedu.address.model.tag.Tag;

/**
* An UI component that displays information of a {@code Person}.
Expand Down Expand Up @@ -53,7 +54,8 @@
address.setText(person.getAddress().toString());
email.setText(person.getEmail().toString());
person.getTags().stream()
.sorted(Comparator.comparing(tag -> tag.tagName))
.sorted(Comparator.comparing(Tag::get))

Check warning on line 57 in src/main/java/seedu/address/ui/PersonCard.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/ui/PersonCard.java#L57

Added line #L57 was not covered by tests
.forEach(tag -> tags.getChildren().add(new Label(tag.get())));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static void showPersonAtIndex(Model model, Index targetIndex) {
assertTrue(targetIndex.getZeroBased() < model.getFilteredPersonList().size());

Person person = model.getFilteredPersonList().get(targetIndex.getZeroBased());
final String[] splitName = person.getName().name.split("\\s+");
final String[] splitName = person.getName().toString().split("\\s+");
model.updateFilteredPersonList(new NameContainsKeywordsPredicate(Arrays.asList(splitName[0])));

assertEquals(1, model.getFilteredPersonList().size());
Expand Down
18 changes: 6 additions & 12 deletions src/test/java/seedu/address/logic/parser/AddCommandParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@
import seedu.address.logic.Messages;
import seedu.address.logic.commands.AddCommand;
import seedu.address.model.person.Person;
import seedu.address.model.person.fields.Address;
import seedu.address.model.person.fields.Email;
import seedu.address.model.person.fields.Name;
import seedu.address.model.person.fields.Phone;
import seedu.address.model.tag.Tag;
import seedu.address.testutil.PersonBuilder;

public class AddCommandParserTest {
Expand Down Expand Up @@ -166,27 +161,26 @@ public void parse_compulsoryFieldMissing_failure() {
public void parse_invalidValue_failure() {
// invalid name
assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Name.MESSAGE_CONSTRAINTS);
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND);

// invalid phone
assertParseFailure(parser, NAME_DESC_BOB + INVALID_PHONE_DESC + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Phone.MESSAGE_CONSTRAINTS);
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND);

// invalid email
assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + INVALID_EMAIL_DESC + ADDRESS_DESC_BOB
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Email.MESSAGE_CONSTRAINTS);
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND);

// invalid address
assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Address.MESSAGE_CONSTRAINTS);
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND);

// invalid tag
assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ INVALID_TAG_DESC + VALID_TAG_FRIEND, Tag.MESSAGE_CONSTRAINTS);
+ INVALID_TAG_DESC + VALID_TAG_FRIEND);

// two invalid values, only first invalid value reported
assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC,
Name.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC);

// non-empty preamble
assertParseFailure(parser, PREAMBLE_NON_EMPTY + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package seedu.address.logic.parser;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import seedu.address.logic.commands.Command;
import seedu.address.logic.parser.exceptions.ParseException;
Expand Down Expand Up @@ -36,4 +37,12 @@ public static void assertParseFailure(Parser<? extends Command> parser, String u
assertEquals(expectedMessage, pe.getMessage());
}
}

/**
* Asserts that the parsing of {@code userInput} by {@code parser} is unsuccessful.
*/
public static void assertParseFailure(Parser<? extends Command> parser, String userInput) {
assertThrows(ParseException.class, () -> parser.parse(userInput));
}

}
26 changes: 10 additions & 16 deletions src/test/java/seedu/address/logic/parser/EditCommandParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@
import seedu.address.logic.Messages;
import seedu.address.logic.commands.EditCommand;
import seedu.address.logic.commands.EditCommand.EditPersonDescriptor;
import seedu.address.model.person.fields.Address;
import seedu.address.model.person.fields.Email;
import seedu.address.model.person.fields.Name;
import seedu.address.model.person.fields.Phone;
import seedu.address.model.tag.Tag;
import seedu.address.testutil.EditPersonDescriptorBuilder;

public class EditCommandParserTest {
Expand Down Expand Up @@ -83,24 +78,23 @@ public void parse_invalidPreamble_failure() {

@Test
public void parse_invalidValue_failure() {
assertParseFailure(parser, "1" + INVALID_NAME_DESC, Name.MESSAGE_CONSTRAINTS); // invalid name
assertParseFailure(parser, "1" + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS); // invalid phone
assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, Email.MESSAGE_CONSTRAINTS); // invalid email
assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, Address.MESSAGE_CONSTRAINTS); // invalid address
assertParseFailure(parser, "1" + INVALID_TAG_DESC, Tag.MESSAGE_CONSTRAINTS); // invalid tag
assertParseFailure(parser, "1" + INVALID_NAME_DESC); // invalid name
assertParseFailure(parser, "1" + INVALID_PHONE_DESC); // invalid phone
assertParseFailure(parser, "1" + INVALID_EMAIL_DESC); // invalid email
assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC); // invalid address
assertParseFailure(parser, "1" + INVALID_TAG_DESC); // invalid tag

// invalid phone followed by valid email
assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, Phone.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY);

// while parsing {@code PREFIX_TAG} alone will reset the tags of the {@code Person} being edited,
// parsing it together with a valid tag results in error
assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_DESC_HUSBAND + TAG_EMPTY, Tag.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_EMPTY + TAG_DESC_HUSBAND, Tag.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, "1" + TAG_EMPTY + TAG_DESC_FRIEND + TAG_DESC_HUSBAND, Tag.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_DESC_HUSBAND + TAG_EMPTY);
assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_EMPTY + TAG_DESC_HUSBAND);
assertParseFailure(parser, "1" + TAG_EMPTY + TAG_DESC_FRIEND + TAG_DESC_HUSBAND);

// multiple invalid values, but only the first invalid value is captured
assertParseFailure(parser, "1" + INVALID_NAME_DESC + INVALID_EMAIL_DESC + VALID_ADDRESS_AMY + VALID_PHONE_AMY,
Name.MESSAGE_CONSTRAINTS);
assertParseFailure(parser, "1" + INVALID_NAME_DESC + INVALID_EMAIL_DESC + VALID_ADDRESS_AMY + VALID_PHONE_AMY);
}

@Test
Expand Down
Loading