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

Commit

Permalink
Use chained exceptions consistently throughout the code base
Browse files Browse the repository at this point in the history
In some places of our code, we respond to an exception by throwing
another exception.

In such a situation, it is good practice to _chain_ the exceptions by
passing the initial exception as a `cause` to the next exception. This
is because it provides more information to higher-level exception
handlers, allowing them to e.g. provide more meaningful stack traces
that show how the exception is handled and transformed as it is
propagated through the call stack.

However, certain places in our code base do not use exception chaining
when they could. These were discovered by grepping the code base for
`\bthrow\b` and seeing if any of the thrown exceptions fit the criteria
described above.

Let's fix them to use chained exceptions.

See https://docs.oracle.com/javase/tutorial/essential/exceptions/chained.html
for more information on chained exceptions.
  • Loading branch information
pyokagan committed Aug 6, 2018
1 parent d7b3f1c commit 69728c1
Show file tree
Hide file tree
Showing 18 changed files with 28 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public CommandResult execute() throws CommandException {
model.commitAddressBook();
return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
} catch (DuplicatePersonException e) {
throw new CommandException(MESSAGE_DUPLICATE_PERSON);
throw new CommandException(MESSAGE_DUPLICATE_PERSON, e);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public CommandResult execute() throws CommandException {
model.deletePerson(personToDelete);
model.commitAddressBook();
} catch (PersonNotFoundException pnfe) {
throw new AssertionError("The target person cannot be missing");
throw new AssertionError("The target person cannot be missing", pnfe);
}

return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ public CommandResult execute() throws CommandException {
try {
model.updatePerson(personToEdit, editedPerson);
} catch (DuplicatePersonException dpe) {
throw new CommandException(MESSAGE_DUPLICATE_PERSON);
throw new CommandException(MESSAGE_DUPLICATE_PERSON, dpe);
} catch (PersonNotFoundException pnfe) {
throw new AssertionError("The target person cannot be missing");
throw new AssertionError("The target person cannot be missing", pnfe);
}
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
model.commitAddressBook();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ public class CommandException extends Exception {
public CommandException(String message) {
super(message);
}

/**
* Constructs a new {@code CommandException} with the specified detail {@code message} and {@code cause}.
*/
public CommandException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public DeleteCommand parse(String args) throws ParseException {
return new DeleteCommand(index);
} catch (ParseException pe) {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE));
String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public EditCommand parse(String args) throws ParseException {
try {
index = ParserUtil.parseIndex(argMultimap.getPreamble());
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE));
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE), pe);
}

EditPersonDescriptor editPersonDescriptor = new EditPersonDescriptor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public SelectCommand parse(String args) throws ParseException {
return new SelectCommand(index);
} catch (ParseException pe) {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, SelectCommand.MESSAGE_USAGE));
String.format(MESSAGE_INVALID_COMMAND_FORMAT, SelectCommand.MESSAGE_USAGE), pe);
}
}
}
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 @@ -54,7 +54,7 @@ public void resetData(ReadOnlyAddressBook newData) {
try {
setPersons(newData.getPersonList());
} catch (DuplicatePersonException e) {
throw new AssertionError("AddressBooks should not have duplicate persons");
throw new AssertionError("AddressBooks should not have duplicate persons", e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/storage/XmlFileStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static void saveDataToFile(Path file, XmlSerializableAddressBook addressB
try {
XmlUtil.saveDataToFile(file, addressBook);
} catch (JAXBException e) {
throw new AssertionError("Unexpected exception " + e.getMessage());
throw new AssertionError("Unexpected exception " + e.getMessage(), e);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/java/seedu/address/TestApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ public AddressBook readStorageAddressBook() {
try {
return new AddressBook(storage.readAddressBook().get());
} catch (DataConversionException dce) {
throw new AssertionError("Data is not in the AddressBook format.");
throw new AssertionError("Data is not in the AddressBook format.", dce);
} catch (IOException ioe) {
throw new AssertionError("Storage file cannot be found.");
throw new AssertionError("Storage file cannot be found.", ioe);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public AddressBookBuilder withPerson(Person person) {
try {
addressBook.addPerson(person);
} catch (DuplicatePersonException dpe) {
throw new IllegalArgumentException("person is expected to be unique.");
throw new IllegalArgumentException("person is expected to be unique.", dpe);
}
return this;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/seedu/address/testutil/Assert.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static void assertThrows(Class<? extends Throwable> expectedException, St
return;
}

throw new AssertionError(errorMessage);
throw new AssertionError(errorMessage, actualException);
}

throw new AssertionError(String.format(
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/seedu/address/testutil/TypicalPersons.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static AddressBook getTypicalAddressBook() {
try {
ab.addPerson(person);
} catch (DuplicatePersonException e) {
throw new AssertionError("not possible");
throw new AssertionError("not possible", e);
}
}
return ab;
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/systemtests/AddCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private void assertCommandSuccess(String command, Person toAdd) {
try {
expectedModel.addPerson(toAdd);
} catch (DuplicatePersonException dpe) {
throw new IllegalArgumentException("toAdd already exists in the model.");
throw new IllegalArgumentException("toAdd already exists in the model.", dpe);
}
String expectedResultMessage = String.format(AddCommand.MESSAGE_SUCCESS, toAdd);

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/systemtests/AddressBookSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ protected void assertSelectedCardChanged(Index expectedSelectedCardIndex) {
try {
expectedUrl = new URL(BrowserPanel.SEARCH_PAGE_URL + selectedCardName.replaceAll(" ", "%20"));
} catch (MalformedURLException mue) {
throw new AssertionError("URL expected to be valid.");
throw new AssertionError("URL expected to be valid.", mue);
}
assertEquals(expectedUrl, getBrowserPanel().getLoadedUrl());

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/systemtests/DeleteCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private Person removePerson(Model model, Index index) {
try {
model.deletePerson(targetPerson);
} catch (PersonNotFoundException pnfe) {
throw new AssertionError("targetPerson is retrieved from model.");
throw new AssertionError("targetPerson is retrieved from model.", pnfe);
}
return targetPerson;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/systemtests/EditCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ private void assertCommandSuccess(String command, Index toEdit, Person editedPer
expectedModel.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
} catch (DuplicatePersonException | PersonNotFoundException e) {
throw new IllegalArgumentException(
"editedPerson is a duplicate in expectedModel, or it isn't found in the model.");
"editedPerson is a duplicate in expectedModel, or it isn't found in the model.", e);
}

assertCommandSuccess(command, expectedModel,
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/systemtests/SystemTestSetupHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public TestApp setupApplication(Supplier<ReadOnlyAddressBook> addressBook, Path
FxToolkit.registerStage(Stage::new);
FxToolkit.setupApplication(() -> testApp = new TestApp(addressBook, saveFileLocation));
} catch (TimeoutException te) {
throw new AssertionError("Application takes too long to set up.");
throw new AssertionError("Application takes too long to set up.", te);
}

return testApp;
Expand Down Expand Up @@ -55,7 +55,7 @@ public MainWindowHandle setupMainWindowHandle() {
});
FxToolkit.showStage();
} catch (TimeoutException te) {
throw new AssertionError("Stage takes too long to set up.");
throw new AssertionError("Stage takes too long to set up.", te);
}

return mainWindowHandle;
Expand All @@ -68,7 +68,7 @@ public void tearDownStage() {
try {
FxToolkit.cleanupStages();
} catch (TimeoutException te) {
throw new AssertionError("Stage takes too long to tear down.");
throw new AssertionError("Stage takes too long to tear down.", te);
}
}
}

0 comments on commit 69728c1

Please sign in to comment.