Skip to content

Commit

Permalink
[#855] Refactor PersonBuilder constructor to use TypicalPersons as ar…
Browse files Browse the repository at this point in the history
…gument (#889)

When creating a Person for testing, some tests specify almost all of
the fields of an existing TypicalPerson, with only a slight variation
in one or two fields. For example, the code below builds on top of
TypicalPerson#AMY with only changes to address and tags:

    toAdd = new PersonBuilder().withName(VALID_NAME_AMY).withPhone(VALID_PHONE_AMY)
            .withEmail(VALID_EMAIL_AMY).withAddress(VALID_ADDRESS_BOB)
            .withTags(VALID_TAG_FRIEND).build();

This is long-winded and unnecessary, as PersonBuilder allows us to start
from a ready-made Person, and modify as necessary. For example, the code
above can be refactored to:

    toAdd = new PersonBuilder(AMY).withAddress(VALID_ADDRESS_BOB).build();

Let's refactor the calls to PersonBuilder to make them more concise.
  • Loading branch information
yamgent authored Jul 25, 2018
2 parents c47a1e3 + 7376502 commit ac37d6e
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 22 deletions.
17 changes: 6 additions & 11 deletions src/test/java/seedu/address/logic/parser/AddCommandParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,16 @@
import static seedu.address.logic.commands.CommandTestUtil.PREAMBLE_WHITESPACE;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_HUSBAND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;
import static seedu.address.testutil.TypicalPersons.AMY;
import static seedu.address.testutil.TypicalPersons.BOB;

import org.junit.Test;

Expand All @@ -47,8 +45,7 @@ public class AddCommandParserTest {

@Test
public void parse_allFieldsPresent_success() {
Person expectedPerson = new PersonBuilder().withName(VALID_NAME_BOB).withPhone(VALID_PHONE_BOB)
.withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB).withTags(VALID_TAG_FRIEND).build();
Person expectedPerson = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND).build();

// whitespace only preamble
assertParseSuccess(parser, PREAMBLE_WHITESPACE + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB
Expand All @@ -71,18 +68,16 @@ public void parse_allFieldsPresent_success() {
+ ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));

// multiple tags - all accepted
Person expectedPersonMultipleTags = new PersonBuilder().withName(VALID_NAME_BOB).withPhone(VALID_PHONE_BOB)
.withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB)
.withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND).build();
Person expectedPersonMultipleTags = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND)
.build();
assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, new AddCommand(expectedPersonMultipleTags));
}

@Test
public void parse_optionalFieldsMissing_success() {
// zero tags
Person expectedPerson = new PersonBuilder().withName(VALID_NAME_AMY).withPhone(VALID_PHONE_AMY)
.withEmail(VALID_EMAIL_AMY).withAddress(VALID_ADDRESS_AMY).withTags().build();
Person expectedPerson = new PersonBuilder(AMY).withTags().build();
assertParseSuccess(parser, NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY,
new AddCommand(expectedPerson));
}
Expand Down
7 changes: 1 addition & 6 deletions src/test/java/systemtests/AddCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@
import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_HUSBAND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
import static seedu.address.testutil.TypicalPersons.ALICE;
import static seedu.address.testutil.TypicalPersons.AMY;
Expand Down Expand Up @@ -79,8 +75,7 @@ public void add() throws Exception {
assertCommandSuccess(command, model, expectedResultMessage);

/* Case: add a person with all fields same as another person in the address book except name -> added */
toAdd = new PersonBuilder().withName(VALID_NAME_BOB).withPhone(VALID_PHONE_AMY).withEmail(VALID_EMAIL_AMY)
.withAddress(VALID_ADDRESS_AMY).withTags(VALID_TAG_FRIEND).build();
toAdd = new PersonBuilder(AMY).withName(VALID_NAME_BOB).build();
command = AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY
+ TAG_DESC_FRIEND;
assertCommandSuccess(command, toAdd);
Expand Down
6 changes: 1 addition & 5 deletions src/test/java/systemtests/EditCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@
import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_HUSBAND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
Expand Down Expand Up @@ -67,8 +64,7 @@ public void edit() throws Exception {
Index index = INDEX_FIRST_PERSON;
String command = " " + EditCommand.COMMAND_WORD + " " + index.getOneBased() + " " + NAME_DESC_BOB + " "
+ PHONE_DESC_BOB + " " + EMAIL_DESC_BOB + " " + ADDRESS_DESC_BOB + " " + TAG_DESC_HUSBAND + " ";
Person editedPerson = new PersonBuilder().withName(VALID_NAME_BOB).withPhone(VALID_PHONE_BOB)
.withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB).withTags(VALID_TAG_HUSBAND).build();
Person editedPerson = new PersonBuilder(BOB).withTags(VALID_TAG_HUSBAND).build();
assertCommandSuccess(command, index, editedPerson);

/* Case: undo editing the last person in the list -> last person restored */
Expand Down

0 comments on commit ac37d6e

Please sign in to comment.