Возвращает исключение анти-шаблон?
У меня есть два простых метода:
public void proceedWhenError() {
Throwable exception = serviceUp();
if (exception == null) {
// do stuff
} else {
logger.debug("Exception happened, but it alright.", exception)
// do stuff
}
}
public void doNotProceedWhenError() {
Throwable exception = serviceUp();
if (exception == null) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Третий метод - частный вспомогательный метод:
private Throwable serviceUp() {
try {
service.connect();
return null;
catch(Exception e) {
return e;
}
}
Мы поговорили с моим коллегой по поводу используемой здесь схемы:
возвращая объект Exception
(или Throwable
) из serviceUp()
.
Первое мнение:
Анти-шаблон использует Исключения для управления рабочим процессом, и мы должны возвращать только boolean из serviceUp()
и никогда не serviceUp()
объектом Exception. Аргументом является то, что использование исключений для управления рабочим процессом является анти-шаблоном.
Второе мнение:
Это хорошо, так как нам нужно иметь дело с объектом впоследствии в двух первых методах по-разному и независимо от того, не возвращает ли возвращаемый объект Exception или boolean рабочий процесс вообще
Считаете ли вы, что 1) или 2) является правильным и особенно, почему? Обратите внимание, что вопрос ТОЛЬКО о методе serviceUp()
и его возвращаемом типе - boolean
vs Exception
object.
Примечание. Я не задаюсь вопросом, следует ли использовать объекты Throwable или Exception.
Ответы
Ответ 1
Анти-шаблон использует исключения для прямого потока только тогда, когда исключение выбрасывается в исключительную ситуацию *. Например, завершение цикла путем исключения исключения, когда вы достигаете конца коллекции, является анти-шаблоном.
С другой стороны, контроль потока с фактическими исключениями является хорошим применением исключений. Если ваш метод встречает исключительную ситуацию, с которой он не может справиться, он должен генерировать исключение, тем самым перенаправляя поток в вызывающем объекте в блок обработчика исключений.
Возвращение "голого" объекта Exception
из метода, а не его броска, безусловно, противоречит интуиции. Если вам нужно сообщить результаты операции вызывающему абоненту, лучше использовать объект состояния, который обертывает всю соответствующую информацию, включая исключение:
public class CallStatus {
private final Exception serviceException;
private final boolean isSuccess;
public static final CallStatus SUCCESS = new CallStatus(null, true);
private CallStatus(Exception e, boolean s) {
serviceException = e;
isSuccess = s;
}
public boolean isSuccess() { return isSuccess; }
public Exception getServiceException() { return serviceException; }
public static CallStatus error(Exception e) {
return new CallStatus(e, false);
}
}
Теперь вызывающий абонент получит CallStatus
из serviceUp
:
CallStatus status = serviceUp();
if (status.isSuccess()) {
... // Do something
} else {
... // Do something else
Exception ex = status.getException();
}
Обратите внимание, что конструктор является закрытым, поэтому serviceUp
либо вернет CallStatus.SUCCESS
либо вызовет CallStatus.error(myException)
.
* Что является исключительным и что не является исключительным, во многом зависит от контекста. Например, нечисловые данные приводят к исключению в Scanner
nextInt
, поскольку он считает такие данные недействительными. Однако одни и те же точные данные не вызывают исключения в методе hasNextInt
, потому что это совершенно верно.
Ответ 2
Второе мнение ("это хорошо") не выполняется. Код не в порядке, потому что возврат исключений вместо их броска не является действительно идиоматическим.
Я также не покупаю первое мнение ("использование исключений для управления рабочим процессом является анти-шаблоном"). service.connect()
выбрасывает исключение, и вы должны реагировать на это исключение - так что это эффективное управление потоком. Возвращение boolean
или какого-либо другого объекта состояния и обработка этого вместо обработки исключения - и думать, что он не управляет потоком, основанным на исключении, является наивным. Еще один недостаток заключается в том, что если вы решите перестроить исключение (завершено в IllegalArgumentException
или что-то еще), у вас больше не будет оригинального исключения. И это очень плохо, когда вы пытаетесь проанализировать, что на самом деле произошло.
Поэтому я бы сделал классику:
- Выбросьте исключение в
serviceUp
. - В методах, вызывающих
serviceUp
: -
try-catch
, log debug и проглатывать исключение, если вы хотите продолжить исключение. -
try-catch
и rethrow исключение, завернутое в другое исключение, предоставляющее больше информации о том, что произошло. В качестве альтернативы просто позвольте исходному исключению распространяться через throws
если вы не можете добавить что-либо существенное.
Самое главное не потерять первоначальное исключение.
Ответ 3
Оба ошибочны
Это хорошо, так как нам нужно иметь дело с объектом впоследствии в двух первых методах по-разному и независимо от того, не возвращает ли возвращаемый объект Exception или boolean рабочий процесс вообще
Это нехорошо. Понятие исключений означает, что они выбрасываются в исключительных случаях. Они должны быть пойманы в месте, где они будут обрабатываться (или, по меньшей мере, повторно выбрасываться после некоторых локальных очистки/регистрации/и т.д.). Они не предназначены для передачи таким образом (то есть в "доменном" коде).
Люди будут смущены. Реальные ошибки могут легко справиться с этим - например, что, если есть какое-то Exception
из какого-то другого источника, кроме сетевого; который вы не ожидали, это действительно плохо (например, какое-то исключение из-за границы, которое вы создали с помощью некоторой ошибки программирования)?
И свежие программисты будут бесконечно путаться и/или копировать этот анти-шаблон в места, где он просто не принадлежит.
Например, недавно коллега реализовал довольно сложный интерфейс (как в интерфейсе между машинами, а не с interface
Java), и он сделал подобную обработку исключений; преобразование исключений в молча игнорируемые варианты (по модулю некоторых сообщений журнала). Излишне говорить, что любое исключение, которое он на самом деле не ожидал, разбило весь беспорядок наихудшим воображаемым способом; наоборот, быстро.
Анти-шаблон использует Исключения для управления рабочим процессом, и мы должны возвращать только boolean из serviceUp() и никогда не являемся объектом Exception. Аргументом является то, что использование исключений для управления рабочим процессом является анти-шаблоном.
Исключения, безусловно, контролируют рабочий процесс в том аспекте, который они часто прерывают, или перенаправляют на сообщение об ошибке, отображаемое пользователю. Вполне возможно, что некоторая часть кода "отключена" из-за исключения; т.е. обработка исключений, безусловно, разрешена где-то еще, а не только на верхнем уровне управления.
Но возвращающие исключения действительно являются анти-шаблонами; никто не ожидает этого, это странно, это приводит к ложным ошибкам (легко игнорировать возвращаемое значение) и т.д. и т.д.
Таким образом, в случае вашего serviceUp()
либо сделать его void
- либо он работает 99% времени, либо генерирует исключение; или сделать его истинным boolean
поскольку вы полностью согласны с тем, что он где-то не сработает. Если вам нужно передать сообщение об ошибке вокруг, сделайте это как String
, или сохранить его где - нибудь в сторону, или что - то, но не использовать его в качестве возвращаемого значения, особенно если вы не намерены throw
его позже.
Простое, стандартное решение
Это решение короче (меньше строк, меньше переменных, меньше, if
), проще, болотно-стандартное и делает именно то, что вы хотели. Простота в обслуживании, легко понять.
public void proceedWhenError() {
try {
serviceUp();
// do stuff (only when no exception)
}
catch (Exception exception) {
logger.debug("Exception happened, but it alright.", exception)
// do stuff (only when exception)
}
}
public void doNotProceedWhenError() {
try {
serviceUp();
// do stuff (only when no exception)
}
catch (Exception exception) {
// do stuff (only when exception)
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
private void serviceUp() {
service.connect();
}
Ответ 4
Я бы вернул ServiceState
который может быть, например, RUNNING
, WAITING
, CLOSED
. Метод будет называться getServiceState
.
enum ServiceState { RUNNING, WAITING, CLOSED; }
Я никогда не видел методов, которые возвращают исключение в результате выполнения. Для меня, когда метод возвращает значение, это означает, что метод завершил свою работу без проблем. Я не хочу извлекать результат и анализировать его с любой ошибкой. Сам результат означает, что никаких провалов не произошло - все пошло так, как планировалось.
С другой стороны, когда метод генерирует исключение, мне нужно проанализировать специальный объект, чтобы выяснить, что пошло не так. Я не разбираю результат, потому что результата нет.
Пример:
public void proceedWhenError() {
final ServiceState state = getServiceState();
if (state != ServiceState.RUNNING) {
logger.debug("The service is not running, but it alright.");
}
// do stuff
}
public void doNotProceedWhenError() {
final ServiceState state = getServiceState();
if (state != ServiceState.RUNNING) {
throw new IllegalStateException("The service is not running...");
}
// do stuff
}
private ServiceState getServiceState() {
try {
service.connect();
return ServiceState.RUNNING;
catch(Exception e) {
// determine the state by parsing the exception
// and return it
return getStateFromException(e);
}
}
Если исключения, брошенные службой, важны и/или обрабатываются в другом месте, он вместе с состоянием может быть сохранен в объект ServiceResponse
:
class ServiceResponse {
private final ServiceState state;
private final Exception exception;
public ServiceResponse(ServiceState state, Exception exception) {
this.state = state;
this.exception = exception;
}
public static ServiceResponse of(ServiceState state) {
return new ServiceResponse(state, null);
}
public static ServiceResponse of(Exception exception) {
return new ServiceResponse(null, exception);
}
public ServiceState getState() {
return state;
}
public Exception getException() {
return exception;
}
}
Теперь, используя ServiceResponse
, эти методы могут выглядеть так:
public void proceedWhenError() {
final ServiceResponse response = getServiceResponse();
final ServiceState state = response.getState();
final Exception exception = response.getException();
if (state != ServiceState.RUNNING) {
logger.debug("The service is not running, but it alright.", exception);
}
// do stuff
}
public void doNotProceedWhenError() {
final ServiceResponse response = getServiceResponse();
final ServiceState state = response.getState();
final Exception exception = response.getException();
if (state != ServiceState.RUNNING) {
throw new IllegalStateException("The service is not running...", exception);
}
// do stuff
}
private ServiceResponse getServiceResponse() {
try {
service.connect();
return ServiceResponse.of(ServiceState.RUNNING);
catch(Exception e) {
// or -> return ServiceResponse.of(e);
return new ServiceResponse(getStateFromException(e), e);
}
}
Ответ 5
возврат Исключения действительно является анти-шаблоном из-за того, что Исключения должны быть зарезервированы для ошибок в выполнении, а не для описания состояния службы.
представьте, есть ли ошибка в коде serviceUp()
который заставляет его бросать NullPointerException
. Теперь представьте, что ошибка находится в сервисе, и те же NullPointerException
выбрасываются из connect()
.
См. Мой вопрос?
Другая причина - изменение требований.
В настоящее время служба имеет два условия: вверх или вниз.
В настоящее время.
Tommorow, у вас будет три условия для обслуживания: вверх, вниз. или работает с предупреждениями. На следующий день вам также понадобится, чтобы метод возвращал информацию об услуге в json.....
Ответ 6
Очевидным когнитивным диссонансом является анти-шаблон. Читатель увидит, что вы используете исключения для управления потоком, и разработчик немедленно попытается перекопировать его, чтобы он этого не сделал.
Мой инстинкт предлагает такой подход, как:
// Use an action name instead of a question here because it IS an action.
private void bringServiceUp() throws Exception {
}
// Encapsulate both a success and a failure into the result.
class Result {
final boolean success;
final Exception failure;
private Result(boolean success, Exception failure) {
this.success = success;
this.failure = failure;
}
Result(boolean success) {
this(success, null);
}
Result(Exception exception) {
this(false, exception);
}
public boolean wasSuccessful() {
return success;
}
public Exception getFailure() {
return failure;
}
}
// No more cognitive dissonance.
private Result tryToBringServiceUp() {
try {
bringServiceUp();
} catch (Exception e) {
return new Result(e);
}
return new Result(true);
}
// Now these two are fine.
public void proceedWhenError() {
Result result = tryToBringServiceUp();
if (result.wasSuccessful()) {
// do stuff
} else {
logger.debug("Exception happened, but it alright.", result.getFailure());
// do stuff
}
}
public void doNotProceedWhenError() throws IllegalStateException {
Result result = tryToBringServiceUp();
if (result.wasSuccessful()) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", result.getFailure());
}
}
Ответ 7
Если вызывающие вызовы метода будут ожидать, что операция может преуспеть или завершиться неудачей и готовы обрабатывать любой случай, тогда метод должен указывать, как правило, через возвращаемое значение, была ли операция неудачной. Если вызывающие абоненты не готовы к эффективному управлению сбоем, то метод, в котором произошел сбой, должен генерировать исключение, а не требовать, чтобы вызывающие пользователи добавляли код для этого.
Одна морщина заключается в том, что некоторые методы будут иметь некоторые вызывающие, которые готовы грациозно обрабатывать отказы, а другие - нет. Мой предпочтительный подход заключался бы в том, чтобы такие методы принимали необязательный обратный вызов, который вызывается в случае отказа; если обратный вызов не указан, по умолчанию должно быть выбрано исключение. Такой подход позволит сэкономить затраты на создание исключения в случаях, когда вызывающий абонент готов к работе с отказом, при этом минимизируя нагрузку на вызывающих абонентов, которые этого не делают. Самая большая трудность с таким дизайном заключается в том, чтобы решить, какие параметры должны выполнять такие обратные вызовы, поскольку изменение таких параметров может оказаться затруднительным.
Ответ 8
Вы не можете сказать, что мнение 2 ложно, поэтому рабочий процесс будет запущен так, как вы хотите его запустить. С логической точки зрения он будет работать так, как хотелось бы, чтобы он исправлялся.
Но это действительно странный и не рекомендованный способ сделать это. Во-первых, потому, что Exception не предназначены для этого (это означает, что вы делаете анти-шаблон). Это особый объект, предназначенный для броска и улова. Так что это странно, скорее использовал его в качестве конструированного, выберете его, чтобы вернуть его, и скорее поймайте его, используя if. Более того, вы (возможно, это пренебрежимо) столкнетесь с проблемой производительности, а не с использованием простого логического значения в качестве флага, вы создаете экземпляр целого объекта.
Наконец, это также не рекомендуется, функция причины должна возвращать что-то, если цель функции - получить что-то (что не относится к вашему случаю). Вы должны перепроектировать его как функцию, которая запускает службу, а затем ничего не возвращает, потому что она не будет вызвана для получения чего-либо, и она вызовет исключение, если произошла ошибка. И если вы хотите знать, работает ли ваша служба, создайте функцию, предназначенную для предоставления вам этой информации, такой как public boolean isServiceStarted()
.
Ответ 9
Существует три (идиоматический) для обработки функций, которые могут выйти из строя во время выполнения.
1. Вернуть логическое значение
Первый - вернуть boolean
. Этот стиль широко используется в API-интерфейсах C и C, таких как PHP. Это приводит - например, в PHP - часто для кода следующим образом:
if (xyz_parse($data) === FALSE)
$error = xyz_last_error();
Недостатки этого очевидны, поэтому он все больше и больше выходит из моды, особенно на языках ООП и языках с обработкой исключений.
2. Используйте объект посредника/состояния/результата
Если вы не используете исключения, у вас все еще есть возможность на языках ООП возвращать объекты, описывающие состояние.
Например, если вы получаете ввод от пользователя и хотите проверить ввод, вы заранее знаете, что можете получить мусор и что результат будет часто не проверяться, и вы можете написать код следующим образом:
ValidationResult result = parser.validate(data);
if (result.isValid())
// business logic
else
error = result.getvalidationError();
3. Использовать исключения
Java делает это, как вы показали с сокетами. Причиной является то, что создание соединения должно быть успешным и только в исключительных обстоятельствах, требующих специальной обработки.
Применение
В вашем случае я бы просто исключил исключение и даже отбросил вспомогательный метод. Ваш вспомогательный метод плохо назван. Он фактически не запрашивает, работает ли служба (как следует из названия), а просто подключается.
Использовать (отмеченные) исключения
Предположим, что соединение - это то, что на самом деле делает ваш метод. В этом случае мы назовем его более метко connectToService
и сделаем его прямое исключение:
public void connectToService() thows IOException {
// yada yada some init stuff, whatever
socket.connect();
}
public void proceedWhenError() {
try {
connectToService();
} else {
logger.debug("Exception happened, but it alright.", exception)
// do stuff
}
}
public void doNotProceedWhenError() throws IllegalStateException {
try {
connectToService();
// do stuff
}
catch(IOException e) {
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Использовать boolean
С другой стороны, возможно, что ваш serviceUp
(который по-прежнему плохо назван и будет называться isServiceUp
) действительно запрашивает, работает ли служба или нет (что могло быть запущено кем-то другим). В этом случае использование логического будет правильным способом
public boolean isServiceUp {
return ...; // query the service however you need
}
Это, конечно, относится к первому решению использования булевых элементов и не очень полезно, когда вам также нужно знать, почему служба не работает.
Использовать объекты-посредники/результат/состояние
Поэтому давайте отменим этот ограниченный подход и переработаем его с использованием объекта посредника/результата/состояния:
class ServiceState {
enum State { CONNECTED, WAITING, CLOSED; }
State state;
IOException exception;
String additionalMessage;
public ServiceState (State state, IOException ex, String msg) {
[...] // ommitted for brevity
}
// getters ommitted for brevity
}
Вспомогательная функция станет getServiceState
:
public ServiceState getServiceState() {
// query the service state however you need & return
}
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
logger.debug("Exception happened, but it alright.", state.getException())
// do stuff
}
}
public void doNotProceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Обратите внимание, что вы также повторяетесь в proceedWhenError
, весь код может быть упрощен до:
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() != State.CONNECTED) {
logger.debug("Exception happened, but it alright.", state.getException())
}
// do stuff
}
Есть немного дебатов, когда нужно использовать второй случай, а когда использовать третий. Некоторые люди считают, что исключения должны быть исключительными и что вы не должны проектировать с возможностью исключений в виду и в значительной степени всегда будете использовать второй вариант. Это нормально. Но мы проверяли исключения на Java, поэтому я не вижу причин не использовать их. Я использую проверенные исключения, когда основное предположение заключается в том, что вызов должен быть успешным (например, с использованием сокета), но сбой возможен, и я использую второй вариант, когда его очень неясно, должен ли вызов быть успешным (например, проверка данных). Но есть разные мнения по этому поводу.
Это также зависит от того, что на самом деле делает ваш вспомогательный метод. isServiceUp
подразумевает, что он запрашивает какое-то состояние, но не меняет его. В этом случае возврат объекта состояния, очевидно, является идиоматическим способом справиться с этим.
Но ваша реализация показывает, что вспомогательный метод подключается к службе. В этом случае, по крайней мере, в java, идиоматический способ справиться с этим был бы броском (отмеченным) исключением, но вы также могли бы оправдать использование объекта результата.
Просто возврат boolean
не рекомендуется, поскольку он слишком ограничен. Если все, что вам нужно знать, работает ли служба или нет (и не заинтересованы в причине), такой метод может быть полезен, хотя (и может быть, за кулисами, реализован как вспомогательный метод, который просто return getServiceState() == State.SUCCESS
).
Анти-шаблон использует Исключения для управления рабочим процессом
Итак, давайте посмотрим, что такое исключение?
Определение. Исключением является событие, которое происходит во время выполнения программы, что нарушает нормальный поток инструкций программы.
Источник: https://docs.oracle.com/javase/tutorial/essential/exceptions/definition.html
Само определение исключения заключается в том, что это событие, которое разрывает (и, следовательно, изменяет) рабочий процесс программы. Если использовать его как определено, это анти-шаблон, тогда вся функция языка сама по себе должна рассматриваться как анти-шаблон. Есть люди, которые считают, что исключения являются плохими, а некоторые имеют для этого некоторые веские аргументы, но в целом исключения рассматриваются как полезные конструкции и рассматриваются как полезные инструменты для обработки исключительных рабочих процессов.
Бросок исключения, конечно, не является анти-шаблоном. С другой стороны, возвращение. Таким образом, да, ваш код - как представлено - не является идиоматическим.
Ответ 10
Это анти-шаблон:
catch(Exception e) {
return e;
}
Единственным разумным оправданием для возврата Throwable
является предоставление подробной информации о Throwable
на быстрый путь, где ожидается отказ, без уплаты цены обработки исключений 1. В качестве бонуса это упрощает преобразование в исключение, если вызывающий абонент не знает, как справиться с этой конкретной ситуацией.
Но по вашему сценарию эта стоимость уже оплачена. Улавливание исключения от имени вашего вызывающего абонента никому не способствует.
1 Педантично прямая стоимость улавливания исключения (поиск подходящего обработчика, разворачивание стека) довольно низок или необходимо делать в любом случае. Большая часть стоимости try
/catch
по сравнению с возвращаемым значением - это случайные действия, такие как построение трассировки стека. Таким образом, независимо от того, будут ли исключенные объекты исключений создавать хорошее хранилище для эффективных данных об ошибках, зависит от того, выполняется ли эта случайная работа в момент построения или время метания. Следовательно, разумно ли возвращать объект исключения, может отличаться между различными управляемыми платформами.
Ответ 11
Как упоминалось в предыдущих комментариях "Исключение - это событие", объект исключения, который мы получаем, является лишь деталью события. Когда исключение попадает в блок "catch" и не перебрасывается, событие завершается. Сообщение о том, что у вас просто объект детали не исключение, хотя объект имеет класс exception/throwable.
Возвращение объекта исключений может быть полезно из-за деталей, хранящихся в нем, но это добавит неоднозначность/путаницу, поскольку вызывающий метод не "обрабатывает исключение и управляет потоком, основанным на исключении". Он просто использует детали возвращаемого объекта для принятия решений.
Поэтому, на мой взгляд, более логичным и менее запутанным способом будет возврат логического /enum на основе исключения, а не просто возврат объекта исключения.