Unnecessary catch block in MainApp and Exceptions in Interfaces
Closed this issue · 0 comments
Eclipse-Dominator commented
In MainApp.java,
try {
addressBookOptional = storage.readAddressBook();
if (!addressBookOptional.isPresent()) {
logger.info("Data file not found. Will be starting with a sample AddressBook");
}
initialData = addressBookOptional.orElseGet(SampleDataUtil::getSampleAddressBook);
} catch (DataConversionException e) {
logger.warning("Data file not in the correct format. Will be starting with an empty AddressBook");
initialData = new AddressBook();
} catch (IOException e) {
logger.warning("Problem while reading from the file. Will be starting with an empty AddressBook");
initialData = new AddressBook();
}
try {
Optional<UserPrefs> prefsOptional = storage.readUserPrefs();
initializedPrefs = prefsOptional.orElse(new UserPrefs());
} catch (DataConversionException e) {
logger.warning("UserPrefs file at " + prefsFilePath + " is not in the correct format. "
+ "Using default user prefs");
initializedPrefs = new UserPrefs();
} catch (IOException e) {
logger.warning("Problem while reading from the file. Will be starting with an empty AddressBook");
initializedPrefs = new UserPrefs();
}
The IOException was due to readUserPrefs()
and readAddressBook()
possibly throwing IOException
suggested by their respective interface files.
However, in the actual implementation of the methods, the IOException
is actually already handled within the functions itself so this means that IOException
is actually never thrown. Thus, I believe it is safe to remove the catch statement for IOException
as well as in the interface.
e.g.
readUserPref()
In StorageManager.java
@Override
public Optional<UserPrefs> readUserPrefs() throws DataConversionException, IOException {
return userPrefsStorage.readUserPrefs();
}
In JsonUserPrefStorage.java
@Override
public Optional<UserPrefs> readUserPrefs() throws DataConversionException {
return readUserPrefs(filePath);
}
/**
* Similar to {@link #readUserPrefs()}
* @param prefsFilePath location of the data. Cannot be null.
* @throws DataConversionException if the file format is not as expected.
*/
public Optional<UserPrefs> readUserPrefs(Path prefsFilePath) throws DataConversionException {
return JsonUtil.readJsonFile(prefsFilePath, UserPrefs.class);
}
In JsonUtil.java
public static <T> Optional<T> readJsonFile(
Path filePath, Class<T> classOfObjectToDeserialize) throws DataConversionException {
requireNonNull(filePath);
if (!Files.exists(filePath)) {
logger.info("Json file " + filePath + " not found");
return Optional.empty();
}
T jsonFile;
try {
jsonFile = deserializeObjectFromJsonFile(filePath, classOfObjectToDeserialize);
} catch (IOException e) {
logger.warning("Error reading from jsonFile file " + filePath + ": " + e);
throw new DataConversionException(e);
}
return Optional.of(jsonFile);
}
readAddressBook()
in StorageManager.java
@Override
public Optional<ReadOnlyAddressBook> readAddressBook() throws DataConversionException, IOException {
return readAddressBook(addressBookStorage.getAddressBookFilePath());
}
@Override
public Optional<ReadOnlyAddressBook> readAddressBook(Path filePath) throws DataConversionException, IOException {
logger.fine("Attempting to read data from file: " + filePath);
return addressBookStorage.readAddressBook(filePath);
}
In JsonAddressBookStorage.java
@Override
public Optional<ReadOnlyAddressBook> readAddressBook() throws DataConversionException {
return readAddressBook(filePath);
}
/**
* Similar to {@link #readAddressBook()}.
*
* @param filePath location of the data. Cannot be null.
* @throws DataConversionException if the file is not in the correct format.
*/
public Optional<ReadOnlyAddressBook> readAddressBook(Path filePath) throws DataConversionException {
requireNonNull(filePath);
Optional<JsonSerializableAddressBook> jsonAddressBook = JsonUtil.readJsonFile(
filePath, JsonSerializableAddressBook.class);
if (!jsonAddressBook.isPresent()) {
return Optional.empty();
}
try {
return Optional.of(jsonAddressBook.get().toModelType());
} catch (IllegalValueException ive) {
logger.info("Illegal values found in " + filePath + ": " + ive.getMessage());
throw new DataConversionException(ive);
}
}
We can see that none of the classes that implemented those interface actually throws IOExceptions