model: make PersonNotFoundException a RuntimeException

PersonNotFoundException is a checked exception. It is thrown by methods
such as `UniquePersonList#remove(Person)`,
`UniquePersonList#setPerson(Person, Person)` etc. to signify that
certain preconditions were not met -- namely that the person provided to
those methods does not exist in the address book/UniquePersonList.

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 PersonNotFoundException
being a checked exception 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
PersonNotFoundException a RuntimeException. 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.

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,7 +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.PersonNotFoundException;
 import seedu.address.model.tag.Tag;
 
 /**
@@ -80,11 +79,7 @@ public CommandResult execute() throws CommandException {
             throw new CommandException(MESSAGE_DUPLICATE_PERSON);
         }
 
-        try {
-            model.updatePerson(personToEdit, editedPerson);
-        } catch (PersonNotFoundException pnfe) {
-            throw new AssertionError("The target person cannot be missing", pnfe);
-        }
+        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,7 +7,6 @@
 import javafx.collections.ObservableList;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.UniquePersonList;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * Wraps all data at the address-book level
@@ -77,21 +76,20 @@ public void addPerson(Person p) {
 
     /**
      * Replaces the given person {@code target} in the list with {@code editedPerson}.
+     * {@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.
-     *
-     * @throws PersonNotFoundException if {@code target} could not be found in the list.
      */
-    public void updatePerson(Person target, Person editedPerson) throws 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,7 +4,6 @@
 
 import javafx.collections.ObservableList;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 /**
  * The API of the Model component.
@@ -24,8 +23,11 @@ public interface Model {
      */
     boolean hasPerson(Person person);
 
-    /** Deletes the given person. */
-    void deletePerson(Person target) throws PersonNotFoundException;
+    /**
+     * Deletes the given person.
+     * The person must exist in the address book.
+     */
+    void deletePerson(Person target);
 
     /**
      * Adds the given person.
@@ -35,11 +37,10 @@ public interface Model {
 
     /**
      * Replaces the given person {@code target} with {@code editedPerson}.
+     * {@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.
-     *
-     * @throws PersonNotFoundException if {@code target} could not be found in the list.
      */
-    void updatePerson(Person target, Person editedPerson) throws 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,7 +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.PersonNotFoundException;
 
 /**
  * Represents the in-memory model of the address book data.
@@ -65,7 +64,7 @@ public synchronized boolean hasPerson(Person person) {
     }
 
     @Override
-    public synchronized void deletePerson(Person target) throws PersonNotFoundException {
+    public synchronized void deletePerson(Person target) {
         versionedAddressBook.removePerson(target);
         indicateAddressBookChanged();
     }
@@ -78,7 +77,7 @@ public synchronized void addPerson(Person person) {
     }
 
     @Override
-    public void updatePerson(Person target, Person editedPerson) throws PersonNotFoundException {
+    public void updatePerson(Person target, Person editedPerson) {
         requireAllNonNull(target, editedPerson);
 
         versionedAddressBook.updatePerson(target, editedPerson);

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

@@ -48,11 +48,10 @@ public void add(Person toAdd) {
 
     /**
      * Replaces the person {@code target} in the list with {@code editedPerson}.
+     * {@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.
-     *
-     * @throws PersonNotFoundException if {@code target} could not be found in the list.
      */
-    public void setPerson(Person target, Person editedPerson) throws PersonNotFoundException {
+    public void setPerson(Person target, Person editedPerson) {
         requireNonNull(editedPerson);
 
         int index = internalList.indexOf(target);
@@ -69,10 +68,9 @@ public void setPerson(Person target, Person editedPerson) throws PersonNotFoundE
 
     /**
      * 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();

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/test/java/seedu/address/logic/commands/CommandTestUtil.java

@@ -19,7 +19,6 @@
 import seedu.address.model.Model;
 import seedu.address.model.person.NameContainsKeywordsPredicate;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 import seedu.address.testutil.EditPersonDescriptorBuilder;
 
 /**
@@ -127,12 +126,8 @@ public static void showPersonAtIndex(Model model, Index targetIndex) {
      */
     public static void deleteFirstPerson(Model model) {
         Person firstPerson = model.getFilteredPersonList().get(0);
-        try {
-            model.deletePerson(firstPerson);
-            model.commitAddressBook();
-        } catch (PersonNotFoundException pnfe) {
-            throw new AssertionError("Person in filtered list must exist in model.", pnfe);
-        }
+        model.deletePerson(firstPerson);
+        model.commitAddressBook();
     }
 
     /**

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

@@ -31,7 +31,7 @@ public class DeleteCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
 
     @Test
-    public void execute_validIndexUnfilteredList_success() throws Exception {
+    public void execute_validIndexUnfilteredList_success() {
         Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
         DeleteCommand deleteCommand = prepareCommand(INDEX_FIRST_PERSON);
 
@@ -53,7 +53,7 @@ public void execute_invalidIndexUnfilteredList_throwsCommandException() {
     }
 
     @Test
-    public void execute_validIndexFilteredList_success() throws Exception {
+    public void execute_validIndexFilteredList_success() {
         showPersonAtIndex(model, INDEX_FIRST_PERSON);
 
         Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());

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

@@ -39,7 +39,7 @@ public class EditCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
 
     @Test
-    public void execute_allFieldsSpecifiedUnfilteredList_success() throws Exception {
+    public void execute_allFieldsSpecifiedUnfilteredList_success() {
         Person editedPerson = new PersonBuilder().build();
         EditPersonDescriptor descriptor = new EditPersonDescriptorBuilder(editedPerson).build();
         EditCommand editCommand = prepareCommand(INDEX_FIRST_PERSON, descriptor);
@@ -54,7 +54,7 @@ public void execute_allFieldsSpecifiedUnfilteredList_success() throws Exception
     }
 
     @Test
-    public void execute_someFieldsSpecifiedUnfilteredList_success() throws Exception {
+    public void execute_someFieldsSpecifiedUnfilteredList_success() {
         Index indexLastPerson = Index.fromOneBased(model.getFilteredPersonList().size());
         Person lastPerson = model.getFilteredPersonList().get(indexLastPerson.getZeroBased());
 
@@ -89,7 +89,7 @@ public void execute_noFieldSpecifiedUnfilteredList_success() {
     }
 
     @Test
-    public void execute_filteredList_success() throws Exception {
+    public void execute_filteredList_success() {
         showPersonAtIndex(model, INDEX_FIRST_PERSON);
 
         Person personInFilteredList = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());

src/test/java/systemtests/DeleteCommandSystemTest.java

@@ -19,7 +19,6 @@
 import seedu.address.logic.commands.UndoCommand;
 import seedu.address.model.Model;
 import seedu.address.model.person.Person;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 
 public class DeleteCommandSystemTest extends AddressBookSystemTest {
 
@@ -118,11 +117,7 @@ public void delete() {
      */
     private Person removePerson(Model model, Index index) {
         Person targetPerson = getPerson(model, index);
-        try {
-            model.deletePerson(targetPerson);
-        } catch (PersonNotFoundException pnfe) {
-            throw new AssertionError("targetPerson is retrieved from model.", pnfe);
-        }
+        model.deletePerson(targetPerson);
         return targetPerson;
     }
 

src/test/java/systemtests/EditCommandSystemTest.java

@@ -44,15 +44,14 @@
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;
-import seedu.address.model.person.exceptions.PersonNotFoundException;
 import seedu.address.model.tag.Tag;
 import seedu.address.testutil.PersonBuilder;
 import seedu.address.testutil.PersonUtil;
 
 public class EditCommandSystemTest extends AddressBookSystemTest {
 
     @Test
-    public void edit() throws Exception {
+    public void edit() {
         Model model = getModel();
 
         /* ----------------- Performing edit operation while an unfiltered list is being shown ---------------------- */
@@ -235,13 +234,8 @@ private void assertCommandSuccess(String command, Index toEdit, Person editedPer
     private void assertCommandSuccess(String command, Index toEdit, Person editedPerson,
             Index expectedSelectedCardIndex) {
         Model expectedModel = getModel();
-        try {
-            expectedModel.updatePerson(
-                    expectedModel.getFilteredPersonList().get(toEdit.getZeroBased()), editedPerson);
-            expectedModel.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
-        } catch (PersonNotFoundException e) {
-            throw new IllegalArgumentException("editedPerson isn't found in the model.", e);
-        }
+        expectedModel.updatePerson(expectedModel.getFilteredPersonList().get(toEdit.getZeroBased()), editedPerson);
+        expectedModel.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
 
         assertCommandSuccess(command, expectedModel,
                 String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson), expectedSelectedCardIndex);