From ad970d90a8c4a9e81680c053e033f5f2255f459d Mon Sep 17 00:00:00 2001 From: Tan Wang Leng Date: Thu, 2 Mar 2017 12:44:20 +0800 Subject: [PATCH 1/3] Change UniquePersonList#add(...) signature to ReadOnlyPerson UniquePersonList#add(Person) directly inserts the input Person object to the internal list. This makes it possible for outside classes to indirectly modify Person objects in the list, making the list unable to guarantee its constraint of uniqueness for each Person. Let's change the signature of UniquePersonList#add(...) from Person to ReadOnlyPerson, so that the inner logic is forced to store a copy of the Person object, preventing outside classes from modifying objects already inserted into the list and violating the list's constraint. --- .../java/seedu/address/model/person/UniquePersonList.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/seedu/address/model/person/UniquePersonList.java b/src/main/java/seedu/address/model/person/UniquePersonList.java index c05ac4da0dd..d71a8b17a33 100644 --- a/src/main/java/seedu/address/model/person/UniquePersonList.java +++ b/src/main/java/seedu/address/model/person/UniquePersonList.java @@ -34,12 +34,12 @@ public boolean contains(ReadOnlyPerson toCheck) { * * @throws DuplicatePersonException if the person to add is a duplicate of an existing person in the list. */ - public void add(Person toAdd) throws DuplicatePersonException { + public void add(ReadOnlyPerson toAdd) throws DuplicatePersonException { assert toAdd != null; if (contains(toAdd)) { throw new DuplicatePersonException(); } - internalList.add(toAdd); + internalList.add(new Person(toAdd)); } /** From 1d9a07fd3469504f545ccc4492b2d810994137d2 Mon Sep 17 00:00:00 2001 From: Tan Wang Leng Date: Thu, 2 Mar 2017 13:36:25 +0800 Subject: [PATCH 2/3] Change AddressBook#addPerson(...) signature to ReadOnlyPerson AddressBook#addPerson(Person) accepts a mutable Person object. Outside classes are able to modify the Person object after inserting it into the address book. Only AddressBook should be able to modify the Person objects that it owns. Let's change addPerson()'s signature from Person to ReadOnlyPerson, so that the internal Person list is forced to duplicate this object, satisfying our new constraint. --- src/main/java/seedu/address/model/AddressBook.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/seedu/address/model/AddressBook.java b/src/main/java/seedu/address/model/AddressBook.java index 2edb60b7853..9a6d477774a 100644 --- a/src/main/java/seedu/address/model/AddressBook.java +++ b/src/main/java/seedu/address/model/AddressBook.java @@ -83,9 +83,10 @@ public void resetData(ReadOnlyAddressBook newData) { * * @throws UniquePersonList.DuplicatePersonException if an equivalent person already exists. */ - public void addPerson(Person p) throws UniquePersonList.DuplicatePersonException { - syncMasterTagListWith(p); - persons.add(p); + public void addPerson(ReadOnlyPerson p) throws UniquePersonList.DuplicatePersonException { + Person newPerson = new Person(p); + syncMasterTagListWith(newPerson); + persons.add(newPerson); } /** From 9e158dd1e06328b723edeeded077f15019ac39d0 Mon Sep 17 00:00:00 2001 From: Tan Wang Leng Date: Thu, 2 Mar 2017 13:50:58 +0800 Subject: [PATCH 3/3] Change Model#addPerson(...) signature to ReadOnlyPerson Model#addPerson(Person) accepts a mutable Person object. Outside classes are able to modify the Person object after inserting it into the model. Only Model should be able to modify the Person objects that it owns. Let's change addPerson()'s signature from Person to ReadOnlyPerson, so that the internal Person list is forced to duplicate this object, satisfying our new constraint. --- src/main/java/seedu/address/model/Model.java | 3 +-- src/main/java/seedu/address/model/ModelManager.java | 3 +-- .../java/seedu/address/logic/commands/AddCommandTest.java | 8 ++++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java index bb376de4471..9caaa01e445 100644 --- a/src/main/java/seedu/address/model/Model.java +++ b/src/main/java/seedu/address/model/Model.java @@ -3,7 +3,6 @@ import java.util.Set; import seedu.address.commons.core.UnmodifiableObservableList; -import seedu.address.model.person.Person; import seedu.address.model.person.ReadOnlyPerson; import seedu.address.model.person.UniquePersonList; import seedu.address.model.person.UniquePersonList.DuplicatePersonException; @@ -22,7 +21,7 @@ public interface Model { void deletePerson(ReadOnlyPerson target) throws UniquePersonList.PersonNotFoundException; /** Adds the given person */ - void addPerson(Person person) throws UniquePersonList.DuplicatePersonException; + void addPerson(ReadOnlyPerson person) throws UniquePersonList.DuplicatePersonException; /** * Updates the person located at {@code filteredPersonListIndex} with {@code editedPerson}. diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index cc45d6a849f..cc3e1fcce82 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -10,7 +10,6 @@ import seedu.address.commons.events.model.AddressBookChangedEvent; import seedu.address.commons.util.CollectionUtil; import seedu.address.commons.util.StringUtil; -import seedu.address.model.person.Person; import seedu.address.model.person.ReadOnlyPerson; import seedu.address.model.person.UniquePersonList; import seedu.address.model.person.UniquePersonList.PersonNotFoundException; @@ -65,7 +64,7 @@ public synchronized void deletePerson(ReadOnlyPerson target) throws PersonNotFou } @Override - public synchronized void addPerson(Person person) throws UniquePersonList.DuplicatePersonException { + public synchronized void addPerson(ReadOnlyPerson person) throws UniquePersonList.DuplicatePersonException { addressBook.addPerson(person); updateFilteredListToShowAll(); indicateAddressBookChanged(); diff --git a/src/test/java/seedu/address/logic/commands/AddCommandTest.java b/src/test/java/seedu/address/logic/commands/AddCommandTest.java index 65bbc09fc6a..677915c081f 100644 --- a/src/test/java/seedu/address/logic/commands/AddCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/AddCommandTest.java @@ -69,7 +69,7 @@ private AddCommand getAddCommandForPerson(Person person, Model model) throws Ill */ private class ModelStub implements Model { @Override - public void addPerson(Person person) throws DuplicatePersonException { + public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException { fail("This method should not be called."); } @@ -117,7 +117,7 @@ public void updateFilteredPersonList(Set keywords) { */ private class ModelStubThrowingDuplicatePersonException extends ModelStub { @Override - public void addPerson(Person person) throws DuplicatePersonException { + public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException { throw new DuplicatePersonException(); } } @@ -129,8 +129,8 @@ private class ModelStubAcceptingPersonAdded extends ModelStub { final ArrayList personsAdded = new ArrayList<>(); @Override - public void addPerson(Person person) throws DuplicatePersonException { - personsAdded.add(person); + public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException { + personsAdded.add(new Person(person)); } }