Skip to content

Commit

Permalink
Merge pull request nus-cs2103-AY2122S1#88 from wyrchris/update-add-co…
Browse files Browse the repository at this point in the history
…mmand

Update add command parser to remove model dependency
  • Loading branch information
wyrchris authored Oct 17, 2021
2 parents d866992 + ce9d961 commit e6539fb
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 51 deletions.
5 changes: 0 additions & 5 deletions src/main/java/seedu/address/logic/LogicManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import javafx.collections.ObservableList;
import seedu.address.commons.core.GuiSettings;
import seedu.address.commons.core.LogsCenter;
import seedu.address.logic.commands.AddCommand;
import seedu.address.logic.commands.Command;
import seedu.address.logic.commands.CommandResult;
import seedu.address.logic.commands.exceptions.CommandException;
Expand Down Expand Up @@ -44,10 +43,6 @@ public CommandResult execute(String commandText) throws CommandException, ParseE

CommandResult commandResult;
Command command = addressBookParser.parseCommand(commandText);
if (command instanceof AddCommand) {
model.getAddressBook().incrementClientCounter();

}

commandResult = command.execute(model);
try {
Expand Down
21 changes: 15 additions & 6 deletions src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
import static seedu.address.logic.parser.CliSyntax.PREFIX_RISKAPPETITE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;

import java.util.Optional;
import java.util.function.Function;

import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.Model;
import seedu.address.model.person.ClientId;
import seedu.address.model.person.Person;

/**
Expand Down Expand Up @@ -48,12 +52,12 @@ public class AddCommand extends Command {
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";

private final Person toAdd;
private final Function<ClientId, Person> toAdd;

/**
* Creates an AddCommand to add the specified {@code Person}
*/
public AddCommand(Person person) {
public AddCommand(Function<ClientId, Person> person) {
requireNonNull(person);
toAdd = person;
}
Expand All @@ -62,18 +66,23 @@ public AddCommand(Person person) {
public CommandResult execute(Model model) throws CommandException {
requireNonNull(model);

if (model.hasPerson(toAdd)) {
String clientCounter = Optional.ofNullable(model.getAddressBook().getClientCounter()).orElse("0");
Person person = toAdd.apply(new ClientId(clientCounter));

if (model.hasPerson(person)) {
throw new CommandException(MESSAGE_DUPLICATE_PERSON);
}

model.addPerson(toAdd);
return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
model.addPerson(person);
model.getAddressBook().incrementClientCounter();
return new CommandResult(String.format(MESSAGE_SUCCESS, person));
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof AddCommand // instanceof handles nulls
&& toAdd.equals(((AddCommand) other).toAdd));
&& toAdd.apply(new ClientId("0")).equals(((AddCommand) other)
.toAdd.apply(new ClientId("0"))));
}
}
25 changes: 3 additions & 22 deletions src/main/java/seedu/address/logic/parser/AddCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
import static seedu.address.logic.parser.CliSyntax.allPrefixLess;

import java.util.Set;
import java.util.function.Function;
import java.util.stream.Stream;

import seedu.address.logic.commands.AddCommand;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.Model;
import seedu.address.model.person.Address;
import seedu.address.model.person.ClientId;
import seedu.address.model.person.CurrentPlan;
Expand All @@ -40,15 +40,6 @@ public class AddCommandParser implements Parser<AddCommand> {
PREFIX_NAME, PREFIX_EMAIL
};

private Model model;

public AddCommandParser() {

}

public AddCommandParser(Model model) {
this.model = model;
}
/**
* Parses the given {@code String} of arguments in the context of the AddCommand
* and returns an AddCommand object for execution.
Expand All @@ -64,16 +55,6 @@ public AddCommand parse(String args) throws ParseException {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
}

String clientCounter;

if (this.model == null) {
clientCounter = "0";
} else {
clientCounter = this.model.getAddressBook().getClientCounter() == null ? "0"
: this.model.getAddressBook().getClientCounter();
}

ClientId clientId = new ClientId(clientCounter);
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
Email email = ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).orElse(Phone.DEFAULT_VALUE));
Expand All @@ -87,8 +68,8 @@ public AddCommand parse(String args) throws ParseException {
.orElse(CurrentPlan.DEFAULT_VALUE));
Set<Tag> tagList = ParserUtil.parseTags(argMultimap.getAllValues(PREFIX_TAG));

Person person = new Person(clientId, name, phone, email, address, riskAppetite, disposableIncome,
currentPlan, lastMet, tagList);
Function<ClientId, Person> person = clientId -> new Person(clientId, name, phone, email, address, riskAppetite,
disposableIncome, currentPlan, lastMet, tagList);

return new AddCommand(person);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public Command parseCommand(String userInput) throws ParseException {
switch (commandWord) {

case AddCommand.COMMAND_WORD:
return new AddCommandParser(model).parse(arguments);
return new AddCommandParser().parse(arguments);

case EditCommand.COMMAND_WORD:
return new EditCommandParser().parse(arguments);
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/seedu/address/model/person/Person.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public Name getName() {
}

public Phone getPhone() {

return phone;
}

Expand Down Expand Up @@ -106,8 +105,7 @@ public boolean isSamePerson(Person otherPerson) {
return true;
}

return otherPerson != null
&& otherPerson.getName().equals(getName())
return otherPerson.getName().equals(getName())
&& otherPerson.getClientId().equals(getClientId())
|| otherPerson.getEmail().equals(getEmail());
}
Expand Down Expand Up @@ -175,5 +173,4 @@ public String toString() {
}
return builder.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess;
import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;

import java.util.function.Function;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import seedu.address.model.Model;
import seedu.address.model.ModelManager;
import seedu.address.model.UserPrefs;
import seedu.address.model.person.ClientId;
import seedu.address.model.person.Person;
import seedu.address.testutil.PersonBuilder;

Expand All @@ -27,19 +30,20 @@ public void setUp() {

@Test
public void execute_newPerson_success() {
Function<ClientId, Person> validPersonFunction = new PersonBuilder().buildFunction();
Person validPerson = new PersonBuilder().build();

Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
expectedModel.addPerson(validPerson);
expectedModel.addPerson(new PersonBuilder().build());

assertCommandSuccess(new AddCommand(validPerson), model,
assertCommandSuccess(new AddCommand(validPersonFunction), model,
String.format(AddCommand.MESSAGE_SUCCESS, validPerson), expectedModel);
}

@Test
public void execute_duplicatePerson_throwsCommandException() {
Person personInList = model.getAddressBook().getPersonList().get(0);
assertCommandFailure(new AddCommand(personInList), model, AddCommand.MESSAGE_DUPLICATE_PERSON);
assertCommandFailure(new AddCommand(x -> personInList), model, AddCommand.MESSAGE_DUPLICATE_PERSON);
}

}
16 changes: 12 additions & 4 deletions src/test/java/seedu/address/logic/commands/AddCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;
import java.util.function.Predicate;

import org.junit.jupiter.api.Test;
Expand All @@ -36,27 +37,29 @@ public void constructor_nullPerson_throwsNullPointerException() {
@Test
public void execute_personAcceptedByModel_addSuccessful() throws Exception {
ModelStubAcceptingPersonAdded modelStub = new ModelStubAcceptingPersonAdded();
Function<ClientId, Person> validPersonFunction = new PersonBuilder().buildFunction();
Person validPerson = new PersonBuilder().build();

CommandResult commandResult = new AddCommand(validPerson).execute(modelStub);
CommandResult commandResult = new AddCommand(validPersonFunction).execute(modelStub);

assertEquals(String.format(AddCommand.MESSAGE_SUCCESS, validPerson), commandResult.getFeedbackToUser());
assertEquals(Arrays.asList(validPerson), modelStub.personsAdded);
}

@Test
public void execute_duplicatePerson_throwsCommandException() {
Function<ClientId, Person> validPersonFunction = new PersonBuilder().buildFunction();
Person validPerson = new PersonBuilder().build();
AddCommand addCommand = new AddCommand(validPerson);
AddCommand addCommand = new AddCommand(validPersonFunction);
ModelStub modelStub = new ModelStubWithPerson(validPerson);

assertThrows(CommandException.class, AddCommand.MESSAGE_DUPLICATE_PERSON, () -> addCommand.execute(modelStub));
}

@Test
public void equals() {
Person alice = new PersonBuilder().withName("Alice").build();
Person bob = new PersonBuilder().withName("Bob").build();
Function<ClientId, Person> alice = new PersonBuilder().withName("Alice").buildFunction();
Function<ClientId, Person> bob = new PersonBuilder().withName("Bob").buildFunction();
AddCommand addAliceCommand = new AddCommand(alice);
AddCommand addBobCommand = new AddCommand(bob);

Expand Down Expand Up @@ -210,6 +213,11 @@ public boolean hasPerson(Person person) {
requireNonNull(person);
return this.person.isSamePerson(person);
}

@Override
public ReadOnlyAddressBook getAddressBook() {
return new AddressBook();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,15 @@
import static seedu.address.testutil.TypicalPersons.BOB;
import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;

import java.util.function.Function;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import seedu.address.logic.commands.AddCommand;
import seedu.address.model.ModelManager;
import seedu.address.model.UserPrefs;
import seedu.address.model.person.ClientId;
import seedu.address.model.person.DisposableIncome;
import seedu.address.model.person.Email;
import seedu.address.model.person.Name;
Expand All @@ -59,7 +62,7 @@
public class AddCommandParserTest {

private ModelManager model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
private AddCommandParser parser = new AddCommandParser(model);
private AddCommandParser parser = new AddCommandParser();

@BeforeEach
public void setUp() {
Expand All @@ -68,7 +71,7 @@ public void setUp() {

@Test
public void parse_allFieldsPresent_success() {
Person expectedPerson = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND).build();
Function<ClientId, Person> expectedPerson = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND).buildFunction();

model.getAddressBook().setClientCounter("10");
// whitespace only preamble
Expand Down Expand Up @@ -130,8 +133,8 @@ public void parse_allFieldsPresent_success() {

model.getAddressBook().setClientCounter("10");
// multiple tags - all accepted
Person expectedPersonMultipleTags = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND)
.build();
Function<ClientId, Person> expectedPersonMultipleTags = new PersonBuilder(BOB)
.withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND).buildFunction();
assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ RISKAPPETITE_DESC_BOB + DISPOSABLEINCOME_DESC_BOB + CURRENTPLAN_DESC_BOB + LASTMET_DESC_BOB
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND, new AddCommand(expectedPersonMultipleTags));
Expand All @@ -141,7 +144,7 @@ public void parse_allFieldsPresent_success() {
public void parse_optionalFieldsMissing_success() {
// zero tags
model.getAddressBook().setClientCounter("9");
Person expectedPerson = new PersonBuilder(AMY).withTags().build();
Function<ClientId, Person> expectedPerson = new PersonBuilder(AMY).withTags().buildFunction();
assertParseSuccess(parser, NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY
+ RISKAPPETITE_DESC_AMY + DISPOSABLEINCOME_DESC_AMY + CURRENTPLAN_DESC_AMY + LASTMET_DESC_AMY,
new AddCommand(expectedPerson));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;
import java.util.function.Predicate;

import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -54,9 +55,10 @@ public void setUp() {

@Test
public void parseCommand_add() throws Exception {
Function<ClientId, Person> personFunction = new PersonBuilder().buildFunction();
Person person = new PersonBuilder().build();
AddCommand command = (AddCommand) parser.parseCommand(PersonUtil.getAddCommand(person));
assertEquals(new AddCommand(person), command);
assertEquals(new AddCommand(personFunction), command);
}

@Test
Expand Down
8 changes: 8 additions & 0 deletions src/test/java/seedu/address/testutil/PersonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;

import seedu.address.model.person.Address;
import seedu.address.model.person.ClientId;
Expand Down Expand Up @@ -162,4 +163,11 @@ public Person build() {
disposableIncome, currentPlan, lastMet, tags);
}

/**
* @return {@code Person} function that we are building
*/
public Function<ClientId, Person> buildFunction() {
return clientId -> new Person(clientId, name, phone, email, address, riskAppetite,
disposableIncome, currentPlan, lastMet, tags);
}
}

0 comments on commit e6539fb

Please sign in to comment.