(Note: this is originally posted as this SO question )
I'm trying to refactor the error logging and exception throwing into a method.
Original code:
private Box retrieveBox() {
BoxParameter param = new BoxParameter("red", 10, 5, 3);
List<Long> boxIds = boxDao.retrieveByParameter(param);
if (boxIds.isEmpty()) {
// A
logger.warn("box not found");
throw new BoxNotFoundException();
}
if (boxIds.size() > 1) {
// B
logger.warn("multiple boxes found");
throw new MultipleBoxesFoundException();
}
Optional<Box> maybe = boxDao.retrieve(boxIds.get(0));
if (maybe.isPresent()) {
return maybe.get();
}
// C
logger.warn("box not found");
throw new BoxNotFoundException();
}
Refactored code:
private Box retrieveBox() {
BoxParameter param = new BoxParameter("red", 10, 5, 3);
List<Long> boxIds = boxDao.retrieveByParameter(param);
if (boxIds.isEmpty()) {
// A
logErrorAndThrowBoxNotFoundException();
}
if (boxIds.size() > 1) {
// B
logErrorAndThrowMultipleBoxesFoundException();
}
Long boxId = boxIds.get(0);
Optional<Box> maybe = boxDao.retrieve(boxIds.get(0));
if (maybe.isPresent()) {
return maybe.get();
}
// C-1
// logger.warn("box not found");
// throw new BoxNotFoundException();
// C-2
logErrorAndThrowBoxNotFoundException();
return null; // needs to be here to make it compile
}
private void logWarningAndThrowMultipleBoxesFoundException() throws MultipleBoxesFoundException {
logger.warn("multiple boxes found");
throw new MultipleBoxesFoundException();
}
private void logWarningAndThrowBoxNotFoundException() throws BoxNotFoundException {
logger.warn("box not found");
throw new BoxNotFoundException();
}
I really dislike the return null
part in C-2, but without it, I'll have to go back to C-1, which defeats the purpose of refactoring.
Is there a better way to do it?
throw logErrorAndThrowBoxNotFoundException
which sounds redundant and maybe you should log the error and then do that in here. \$\endgroup\$