model: convert checked exceptions to runtime exceptions (#896)

DuplicatePersonException and PersonNotFoundException are checked
exceptions. They are thrown by methods to signify that their relevant
preconditions were violated.

Using checked exceptions for such a purpose is slightly useful in that
it will force callers to handle the case where the above preconditions
are not met, such as when the methods are called with invalid user
input.

However, it imposes a HUGE cost on callers where the preconditions are
already known to be met (e.g. in test code, or when the user input has
already been validated before hand). In such a case, callers are forced
to add try-catch blocks around the method call even if they know that
the exception will never be thrown, bloating up the code. It is also
impossible to test the catch blocks as well since correct code will
ensure that the precondition holds and thus the exception will never be
thrown, leading to reduced code coverage.

Checked exceptions also don't work very well with the Java Streams API,
since the API doesn't accept lambdas which could throw checked
exceptions.

In AB-4, the amount of code which benefits from DuplicatePersonException
and PersonNotFoundException being checked exceptions is much smaller
than the amount of code which is negatively impacted.

As such, let's make the tradeoff in the other direction, by making
DuplicatePersonException and PersonNotFoundException runtime exceptions.
New callers _could_ forget to check that the preconditions hold before
calling the methods in question (although test cases should catch that),
but this is balanced out by the huge benefit of having more concise and
testable code.

  [1/11] Remove redundant throws clauses
  [2/11] model: expose hasPerson(Person) functionality
  [3/11] AddCommand: explicitly perform duplicate person check
  [4/11] EditCommand: explicitly perform duplicate person check
  [5/11] model: make DuplicatePersonException a RuntimeException
  [6/11] model: make PersonNotFoundException a RuntimeException
  [7/11] AddressBookSystemTest: remove unnecessary catch block
  [8/11] commons: remove DuplicateDataException
  [9/11] UniquePersonList: check for null arguments more thoroughly
  [10/11] UniquePersonListTest: move to `seedu.address.model.person` package
  [11/11] UniquePersonListTest: add some more tests

src/main/java/seedu/address/commons/util/JsonUtil.java

@@ -120,7 +120,7 @@ protected LevelDeserializer(Class<?> vc) {
         }
 
         @Override
-        protected Level _deserialize(String value, DeserializationContext ctxt) throws IOException {
+        protected Level _deserialize(String value, DeserializationContext ctxt) {
             return getLoggingLevel(value);
         }
 

src/main/java/seedu/address/logic/commands/AddCommand.java

@@ -9,7 +9,6 @@
 
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
 
 /**
  * Adds a person to the address book.
@@ -49,14 +48,14 @@ public AddCommand(Person person) {
     @Override
     public CommandResult execute() throws CommandException {
         requireNonNull(model);
-        try {
-            model.addPerson(toAdd);
-            model.commitAddressBook();
-            return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
-        } catch (DuplicatePersonException e) {
-            throw new CommandException(MESSAGE_DUPLICATE_PERSON, e);
+
+        if (model.hasPerson(toAdd)) {
+            throw new CommandException(MESSAGE_DUPLICATE_PERSON);
         }
 
+        model.addPerson(toAdd);
+        model.commitAddressBook();
+        return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
     }
 
     @Override

src/main/java/seedu/address/logic/commands/DeleteCommand.java

@@ -6,7 +6,6 @@
 import seedu.address.commons.core.index.Index;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * Deletes a person identified using it's displayed index from the address book.
@@ -37,14 +36,8 @@ public CommandResult execute() throws CommandException {
         }
 
         Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
-
-        try {
-            model.deletePerson(personToDelete);
-            model.commitAddressBook();
-        } catch (PersonNotFoundException pnfe) {
-            throw new AssertionError("The target person cannot be missing", pnfe);
-        }
-
+        model.deletePerson(personToDelete);
+        model.commitAddressBook();
         return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
     }
 

src/main/java/seedu/address/logic/commands/EditCommand.java

@@ -23,8 +23,6 @@
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 import seedu.address.model.tag.Tag;
 
 /**
@@ -77,13 +75,11 @@ public CommandResult execute() throws CommandException {
         Person personToEdit = lastShownList.get(index.getZeroBased());
         Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);
 
-        try {
-            model.updatePerson(personToEdit, editedPerson);
-        } catch (DuplicatePersonException dpe) {
-            throw new CommandException(MESSAGE_DUPLICATE_PERSON, dpe);
-        } catch (PersonNotFoundException pnfe) {
-            throw new AssertionError("The target person cannot be missing", pnfe);
+        if (!personToEdit.isSamePerson(editedPerson) && model.hasPerson(editedPerson)) {
+            throw new CommandException(MESSAGE_DUPLICATE_PERSON);
         }
+
+        model.updatePerson(personToEdit, editedPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
         model.commitAddressBook();
         return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, editedPerson));

src/main/java/seedu/address/model/AddressBook.java

@@ -7,8 +7,6 @@
 import javafx.collections.ObservableList;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.UniquePersonList;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * Wraps all data at the address-book level
@@ -41,7 +39,11 @@ public AddressBook(ReadOnlyAddressBook toBeCopied) {
 
     //// list overwrite operations
 
-    public void setPersons(List<Person> persons) throws DuplicatePersonException {
+    /**
+     * Replaces the contents of the person list with {@code persons}.
+     * {@code persons} must not contain duplicate persons.
+     */
+    public void setPersons(List<Person> persons) {
         this.persons.setPersons(persons);
     }
 
@@ -51,43 +53,43 @@ public void setPersons(List<Person> persons) throws DuplicatePersonException {
     public void resetData(ReadOnlyAddressBook newData) {
         requireNonNull(newData);
 
-        try {
-            setPersons(newData.getPersonList());
-        } catch (DuplicatePersonException e) {
-            throw new AssertionError("AddressBooks should not have duplicate persons", e);
-        }
+        setPersons(newData.getPersonList());
     }
 
     //// person-level operations
 
+    /**
+     * Returns true if a person with the same identity as {@code person} exists in the address book.
+     */
+    public boolean hasPerson(Person person) {
+        requireNonNull(person);
+        return persons.contains(person);
+    }
+
     /**
      * Adds a person to the address book.
-     *
-     * @throws DuplicatePersonException if an equivalent person already exists.
+     * The person must not already exist in the address book.
      */
-    public void addPerson(Person p) throws DuplicatePersonException {
+    public void addPerson(Person p) {
         persons.add(p);
     }
 
     /**
      * Replaces the given 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 PersonNotFoundException if {@code target} could not be found in the list.
+     * {@code target} must exist in the address book.
+     * The person identity of {@code editedPerson} must not be the same as another existing person in the address book.
      */
-    public void updatePerson(Person target, Person editedPerson)
-            throws DuplicatePersonException, PersonNotFoundException {
+    public void updatePerson(Person target, Person editedPerson) {
         requireNonNull(editedPerson);
 
         persons.setPerson(target, editedPerson);
     }
 
     /**
      * Removes {@code key} from this {@code AddressBook}.
-     * @throws PersonNotFoundException if the {@code key} is not in this {@code AddressBook}.
+     * {@code key} must exist in the address book.
      */
-    public void removePerson(Person key) throws PersonNotFoundException {
+    public void removePerson(Person key) {
         persons.remove(key);
     }
 

src/main/java/seedu/address/model/Model.java

@@ -4,8 +4,6 @@
 
 import javafx.collections.ObservableList;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * The API of the Model component.
@@ -20,21 +18,29 @@ public interface Model {
     /** Returns the AddressBook */
     ReadOnlyAddressBook getAddressBook();
 
-    /** Deletes the given person. */
-    void deletePerson(Person target) throws PersonNotFoundException;
+    /**
+     * Returns true if a person with the same identity as {@code person} exists in the address book.
+     */
+    boolean hasPerson(Person person);
 
-    /** Adds the given person */
-    void addPerson(Person person) throws DuplicatePersonException;
+    /**
+     * Deletes the given person.
+     * The person must exist in the address book.
+     */
+    void deletePerson(Person target);
+
+    /**
+     * Adds the given person.
+     * {@code person} must not already exist in the address book.
+     */
+    void addPerson(Person person);
 
     /**
      * Replaces the given person {@code target} 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 PersonNotFoundException if {@code target} could not be found in the list.
+     * {@code target} must exist in the address book.
+     * The person identity of {@code editedPerson} must not be the same as another existing person in the address book.
      */
-    void updatePerson(Person target, Person editedPerson)
-            throws DuplicatePersonException, PersonNotFoundException;
+    void updatePerson(Person target, Person editedPerson);
 
     /** Returns an unmodifiable view of the filtered person list */
     ObservableList<Person> getFilteredPersonList();

src/main/java/seedu/address/model/ModelManager.java

@@ -13,8 +13,6 @@
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.commons.events.model.AddressBookChangedEvent;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * Represents the in-memory model of the address book data.
@@ -60,21 +58,26 @@ private void indicateAddressBookChanged() {
     }
 
     @Override
-    public synchronized void deletePerson(Person target) throws PersonNotFoundException {
+    public synchronized boolean hasPerson(Person person) {
+        requireNonNull(person);
+        return versionedAddressBook.hasPerson(person);
+    }
+
+    @Override
+    public synchronized void deletePerson(Person target) {
         versionedAddressBook.removePerson(target);
         indicateAddressBookChanged();
     }
 
     @Override
-    public synchronized void addPerson(Person person) throws DuplicatePersonException {
+    public synchronized void addPerson(Person person) {
         versionedAddressBook.addPerson(person);
         updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
         indicateAddressBookChanged();
     }
 
     @Override
-    public void updatePerson(Person target, Person editedPerson)
-            throws DuplicatePersonException, PersonNotFoundException {
+    public void updatePerson(Person target, Person editedPerson) {
         requireAllNonNull(target, editedPerson);
 
         versionedAddressBook.updatePerson(target, editedPerson);

src/main/java/seedu/address/model/person/UniquePersonList.java

@@ -36,10 +36,9 @@ public boolean contains(Person toCheck) {
 
     /**
      * Adds a person to the list.
-     *
-     * @throws DuplicatePersonException if the person to add is a duplicate of an existing person in the list.
+     * The person must not already exist in the list.
      */
-    public void add(Person toAdd) throws DuplicatePersonException {
+    public void add(Person toAdd) {
         requireNonNull(toAdd);
         if (contains(toAdd)) {
             throw new DuplicatePersonException();
@@ -49,13 +48,11 @@ public void add(Person toAdd) throws DuplicatePersonException {
 
     /**
      * Replaces the person {@code target} in the list with {@code editedPerson}.
-     *
-     * @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.
+     * {@code target} must exist in the list.
+     * The person identity of {@code editedPerson} must not be the same as another existing person in the list.
      */
-    public void setPerson(Person target, Person editedPerson)
-            throws DuplicatePersonException, PersonNotFoundException {
-        requireNonNull(editedPerson);
+    public void setPerson(Person target, Person editedPerson) {
+        requireAllNonNull(target, editedPerson);
 
         int index = internalList.indexOf(target);
         if (index == -1) {
@@ -71,21 +68,25 @@ public void setPerson(Person target, Person editedPerson)
 
     /**
      * Removes the equivalent person from the list.
-     *
-     * @throws PersonNotFoundException if no such person could be found in the list.
+     * The person must exist in the list.
      */
-    public void remove(Person toRemove) throws PersonNotFoundException {
+    public void remove(Person toRemove) {
         requireNonNull(toRemove);
         if (!internalList.remove(toRemove)) {
             throw new PersonNotFoundException();
         }
     }
 
     public void setPersons(UniquePersonList replacement) {
+        requireNonNull(replacement);
         this.internalList.setAll(replacement.internalList);
     }
 
-    public void setPersons(List<Person> persons) throws DuplicatePersonException {
+    /**
+     * Replaces the contents of this list with {@code persons}.
+     * {@code persons} must not contain duplicate persons.
+     */
+    public void setPersons(List<Person> persons) {
         requireAllNonNull(persons);
         if (!personsAreUnique(persons)) {
             throw new DuplicatePersonException();

src/main/java/seedu/address/model/person/exceptions/DuplicatePersonException.java

@@ -1,12 +1,10 @@
 package seedu.address.model.person.exceptions;
 
-import seedu.address.commons.exceptions.DuplicateDataException;
-
 /**
  * Signals that the operation will result in duplicate Persons (Persons are considered duplicates if they have the same
  * identity).
  */
-public class DuplicatePersonException extends DuplicateDataException {
+public class DuplicatePersonException extends RuntimeException {
     public DuplicatePersonException() {
         super("Operation would result in duplicate persons");
     }

src/main/java/seedu/address/model/person/exceptions/PersonNotFoundException.java

@@ -3,4 +3,4 @@
 /**
  * Signals that the operation is unable to find the specified person.
  */
-public class PersonNotFoundException extends Exception {}
+public class PersonNotFoundException extends RuntimeException {}

src/main/java/seedu/address/model/util/SampleDataUtil.java

@@ -11,7 +11,6 @@
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;
-import seedu.address.model.person.exceptions.DuplicatePersonException;
 import seedu.address.model.tag.Tag;
 
 /**
@@ -42,15 +41,11 @@ public static Person[] getSamplePersons() {
     }
 
     public static ReadOnlyAddressBook getSampleAddressBook() {
-        try {
-            AddressBook sampleAb = new AddressBook();
-            for (Person samplePerson : getSamplePersons()) {
-                sampleAb.addPerson(samplePerson);
-            }
-            return sampleAb;
-        } catch (DuplicatePersonException e) {
-            throw new AssertionError("sample data cannot contain duplicate persons", e);
+        AddressBook sampleAb = new AddressBook();
+        for (Person samplePerson : getSamplePersons()) {
+            sampleAb.addPerson(samplePerson);
         }
+        return sampleAb;
     }
 
     /**

src/main/java/seedu/address/storage/JsonUserPrefsStorage.java

@@ -25,7 +25,7 @@ public Path getUserPrefsFilePath() {
     }
 
     @Override
-    public Optional<UserPrefs> readUserPrefs() throws DataConversionException, IOException {
+    public Optional<UserPrefs> readUserPrefs() throws DataConversionException {
         return readUserPrefs(filePath);
     }
 

src/test/java/seedu/address/logic/commands/AddCommandIntegrationTest.java

@@ -27,7 +27,7 @@ public void setUp() {
     }
 
     @Test
-    public void execute_newPerson_success() throws Exception {
+    public void execute_newPerson_success() {
         Person validPerson = new PersonBuilder().build();
 
         Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());