Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Commit

Permalink
[#621] UniquePersonList: Simplify logic of updatePerson(...) (#616)
Browse files Browse the repository at this point in the history
This method retrieves the person in the list to be updated, and calls
Person#resetData(ReadOnlyPerson) to update the values of that person.

We do not need to retrieve the person in the list to be updated, to
update the list accordingly.

Let’s simplify the logic by setting the updated Person object in place
of the person to be updated.
  • Loading branch information
Zhiyuan-Amos authored Aug 8, 2017
2 parents a60d16b + 895430f commit ecdd485
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public CommandResult executeUndoableCommand() throws CommandException {
throw new AssertionError("The target person cannot be missing");
}
model.updateFilteredListToShowAll();
return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, personToEdit));
return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/model/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedReadOnlyPer
// TODO: the tags master list will be updated even though the below line fails.
// This can cause the tags master list to have additional tags that are not tagged to any person
// in the person list.
persons.updatePerson(target, editedPerson);
persons.setPerson(target, editedPerson);
}

/**
Expand Down
13 changes: 0 additions & 13 deletions src/main/java/seedu/address/model/person/Person.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,19 +122,6 @@ public void setTags(Set<Tag> replacement) {
tags.set(new UniqueTagList(replacement));
}

/**
* Updates this person with the details of {@code replacement}.
*/
public void resetData(ReadOnlyPerson replacement) {
requireNonNull(replacement);

this.setName(replacement.getName());
this.setPhone(replacement.getPhone());
this.setEmail(replacement.getEmail());
this.setAddress(replacement.getAddress());
this.setTags(replacement.getTags());
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/seedu/address/model/person/UniquePersonList.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@ public void add(ReadOnlyPerson toAdd) throws DuplicatePersonException {
/**
* Replaces the person {@code target} in the list with {@code editedPerson}.
*
* @throws DuplicatePersonException if updating the person's details causes the person to be equivalent to
* another existing person in the list.
* @throws DuplicatePersonException if the replacement is equivalent to another existing person in the list.
* @throws PersonNotFoundException if {@code target} could not be found in the list.
*/
public void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedPerson)
public void setPerson(ReadOnlyPerson target, ReadOnlyPerson editedPerson)
throws DuplicatePersonException, PersonNotFoundException {
requireNonNull(editedPerson);

Expand All @@ -61,12 +60,11 @@ public void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedPerson)
throw new PersonNotFoundException();
}

Person personToUpdate = internalList.get(index);
if (!personToUpdate.equals(editedPerson) && internalList.contains(editedPerson)) {
if (!target.equals(editedPerson) && internalList.contains(editedPerson)) {
throw new DuplicatePersonException();
}

personToUpdate.resetData(editedPerson);
internalList.set(index, new Person(editedPerson));
}

/**
Expand Down

0 comments on commit ecdd485

Please sign in to comment.