Почему метод не должен выдавать несколько типов проверенных исключений?

47

Мы используем SonarQube для анализа нашего Java-кода, и у него есть следующее правило (установлено как критическое):

Публичные методы должны выдавать не более одного проверенного исключения

Использование проверенных исключений вынуждает вызывающие методы обращаться с ошибками, распространяя их или обрабатывая. Это делает эти исключения полностью частью API метода.

Чтобы обеспечить разумную сложность для вызывающих, методы не должны вызывать более одного вида проверенных исключений ».

Еще один бит в сонаре имеет это :

Публичные методы должны выдавать не более одного проверенного исключения

Использование проверенных исключений вынуждает вызывающие методы обращаться с ошибками, распространяя их или обрабатывая. Это делает эти исключения полностью частью API метода.

Чтобы сохранить сложность для вызывающих, методы не должны вызывать более одного вида проверенных исключений.

Следующий код:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

должны быть преобразованы в:

public void delete() throws SomeApplicationLevelException {  // Compliant
    /* ... */
}

Переопределяющие методы не проверяются этим правилом и могут генерировать несколько проверенных исключений.

Я никогда не сталкивался с этим правилом / рекомендацией в своих чтениях по обработке исключений и пытался найти некоторые стандарты, обсуждения и т. Д. По этой теме. Единственное, что я нашел, это из CodeRach: сколько исключений должен вызывать метод?

Это хорошо принятый стандарт?

sdoca
источник
7
Что вы думаете? Объяснение, которое вы цитировали от SonarQube, кажется разумным; есть ли у вас основания сомневаться в этом?
Роберт Харви
3
Я написал много кода, который генерирует более одного исключения, и использовал много библиотек, которые также генерируют более одного исключения. Кроме того, в книгах / статьях по обработке исключений тема ограничения числа создаваемых исключений обычно не поднимается. Но во многих примерах показано, как бросать / ловить множественное число дает неявное одобрение практике. Таким образом, я обнаружил, что правило удивительно, и хотел больше изучить лучшие практики / философию обработки исключений, а не только основные практические примеры.
sdoca

Ответы:

32

Давайте рассмотрим ситуацию, когда у вас есть код:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

Опасность здесь в том, что код, который вы пишете для вызова, delete()будет выглядеть так:

try {
  foo.delete()
} catch (Exception e) {
  /* ... */
}

Это тоже плохо. И это будет поймано с другим правилом, что флаги перехватывают базовый класс Exception.

Ключ в том, чтобы не писать код, который заставляет вас хотеть писать плохой код в другом месте.

Правило, с которым вы сталкиваетесь, является довольно распространенным. Checkstyle имеет это в своих правилах дизайна:

ThrowsCount

Restricts выбрасывает операторы на указанное количество (1 по умолчанию).

Обоснование: исключения являются частью интерфейса метода. Объявление метода для выброса слишком большого количества исключений с разными корнями делает обременительной обработку исключений и приводит к плохим практикам программирования, таким как написание кода, такого как catch (Exception ex). Эта проверка вынуждает разработчиков помещать исключения в иерархию, так что в простейшем случае вызывающая сторона должна проверять только один тип исключений, но любые подклассы могут быть специально перехвачены при необходимости.

Это точно описывает проблему, и в чем проблема, и почему вы не должны это делать. Это общепринятый стандарт, который многие инструменты статического анализа будут определять и отмечать.

И хотя вы можете делать это в соответствии с языковым дизайном, и могут быть моменты, когда это правильно делать, это то, что вы должны увидеть и сразу же сказать: «Почему я это делаю?» Это может быть приемлемо для внутреннего кода, где каждый достаточно дисциплинирован, чтобы никогда catch (Exception e) {}, но чаще всего я видел, как люди срезают углы, особенно во внутренних ситуациях.

Не заставляйте людей, использующих ваш класс, писать плохой код.


Я должен отметить, что важность этого уменьшается в Java SE 7 и более поздних версиях, потому что один оператор catch может перехватывать несколько исключений ( Catching Multiple Exception Type and Rethrowing Exception с улучшенной проверкой типов из Oracle).

В Java 6 и более ранних версиях вы получите код, который выглядит следующим образом:

public void delete() throws IOException, SQLException {
  /* ... */
}

а также

try {
  foo.delete()
} catch (IOException ex) {
     logger.log(ex);
     throw ex;
} catch (SQLException ex) {
     logger.log(ex);
     throw ex;
}

или же

try {
    foo.delete()
} catch (Exception ex) {
    logger.log(ex);
    throw ex;
}

Ни один из этих вариантов с Java 6 не идеален. Первый подход нарушает СУХОЙ . Несколько блоков делают одно и то же, снова и снова - по одному разу для каждого исключения. Вы хотите записать исключение и сбросить его? Хорошо. Те же строки кода для каждого исключения.

Второй вариант хуже по нескольким причинам. Во-первых, это означает, что вы ловите все исключения. Нулевой указатель попадает туда (и не должен). Кроме того, вы перебрасываете значение, Exceptionкоторое означает, что сигнатура метода будет таким, deleteSomething() throws Exceptionчто просто создаст беспорядок в стеке, так как люди, использующие ваш код, теперь вынуждены это делать catch(Exception e).

С Java 7 это не так важно, потому что вместо этого вы можете сделать:

catch (IOException|SQLException ex) {
    logger.log(ex);
    throw ex;
}

Кроме того, проверка типов, если таковые имеются, перехватывает типы создаваемых исключений:

public void rethrowException(String exceptionName)
throws IOException, SQLException {
    try {
        foo.delete();
    } catch (Exception e) {
        throw e;
    }
}

Проверка типов распознает, что eмогут быть только типы IOExceptionили SQLException. Я до сих пор не испытываю особого энтузиазма по поводу использования этого стиля, но он не вызывает такого плохого кода, как это было в Java 6 (где это заставит вас иметь сигнатуру метода как суперкласс, который расширяют исключения).

Несмотря на все эти изменения, многие инструменты статического анализа (Sonar, PMD, Checkstyle) все еще применяют руководства в стиле Java 6. Это не плохо. Я склонен согласиться с предупреждением о том, что они по- прежнему будут применяться, но вы можете изменить их приоритет на майор или минор в зависимости от того, как ваша команда их расставляет по приоритетам.

Если исключения должны быть проверены или незарегистрированный ... это вопрос г р е с т дебаты , которые можно легко найти бесчисленное количество сообщений в блоге , принимая каждую сторону аргумента. Однако, если вы работаете с проверенными исключениями, вам, вероятно, следует избегать создания нескольких типов, по крайней мере, в Java 6.

other_paul
источник
Принимая это как ответ, так как он отвечает на мой вопрос о том, что это хорошо принятый стандарт. Тем не менее, я все еще читаю различные обсуждения про / против, начиная со ссылки Panzercrisis, приведенной в его ответе, чтобы определить, каким будет мой стандарт.
sdoca
«Средство проверки типов распознает, что e может иметь только типы IOException или SQLException.»: Что это значит? Что происходит, когда выбрасывается исключение другого типа foo.delete()? Это все еще пойман и переброшен?
Джорджио
@Giorgio Будет ошибкой времени компиляции, если deleteв этом примере будет выбрано проверенное исключение, кроме IOException или SQLException. Ключевой момент, который я пытался сделать, заключается в том, что метод, вызывающий rethrowException, все равно получит тип исключения в Java 7. В Java 6 все это сводится к общему Exceptionтипу, что делает статический анализ и другие кодеры печальными.
Понимаю. Это кажется мне немного запутанным. Я нашел бы более интуитивным запретить catch (Exception e)и заставить его быть catch (IOException e)или catch (SQLException e)вместо этого.
Джорджио
@Giorgio Это шаг вперед по сравнению с Java 6, чтобы попытаться упростить написание хорошего кода. К сожалению, возможность написания плохого кода будет у нас надолго. Помните, что Java 7 может сделать catch(IOException|SQLException ex)вместо этого. Но, если вы просто собираетесь перебрасывать исключение, позволить средству проверки типов распространять фактический тип исключения, упрощая код, не так уж плохо.
22

Причина, по которой вы в идеале захотите выбросить только один тип исключений, заключается в том, что в противном случае, скорее всего, нарушаются принципы единой инверсии ответственности и зависимости . Давайте использовать пример для демонстрации.

Допустим, у нас есть метод, который извлекает данные из постоянства, а постоянство - это набор файлов. Поскольку мы имеем дело с файлами, мы можем иметь FileNotFoundException:

public String getData(int id) throws FileNotFoundException

Теперь у нас есть изменения в требованиях, и наши данные поступают из базы данных. Вместо FileNotFoundException(поскольку мы не имеем дело с файлами), мы теперь бросаем SQLException:

public String getData(int id) throws SQLException

Теперь нам нужно пройти через весь код, который использует наш метод, и изменить исключение, которое мы должны проверить, иначе код не скомпилируется. Если наш метод вызывается повсюду, это может многое изменить / изменить других. Это занимает много времени, и люди не будут счастливы.

Инверсия зависимостей говорит, что мы действительно не должны выбрасывать ни одно из этих исключений, потому что они раскрывают внутренние детали реализации, над которыми мы работаем. Вызывающий код должен знать, какой тип персистентности мы используем, когда действительно нужно беспокоиться о том, можно ли извлечь запись. Вместо этого мы должны сгенерировать исключение, которое передает ошибку на том же уровне абстракции, который мы выставляем через наш API:

public String getData(int id) throws InvalidRecordException

Теперь, если мы изменим внутреннюю реализацию, мы можем просто обернуть это исключение в InvalidRecordExceptionи передать его (или не обернуть, а просто выбросить новое InvalidRecordException). Внешний код не знает или не заботится, какой тип сохранения используется. Это все заключено в капсулу.


Что касается Единой Ответственности, нам нужно подумать о коде, который генерирует несколько не связанных исключений. Допустим, у нас есть следующий метод:

public Record parseFile(String filename) throws IOException, ParseException

Что мы можем сказать об этом методе? Из подписи мы можем сказать, что он открывает файл и анализирует его. Когда мы видим соединение, такое как «и» или «или» в описании метода, мы знаем, что он делает больше, чем одну вещь; у него более одной ответственности . Трудно управлять методами с более чем одной ответственностью, поскольку они могут измениться, если какая-либо из обязанностей изменится. Вместо этого мы должны разбить методы, чтобы они имели единственную ответственность:

public String readFile(String filename) throws IOException
public Record parse(String data) throws ParseException

Мы сняли ответственность за чтение файла с ответственности за анализ данных. Одним из побочных эффектов этого является то, что теперь мы можем передавать любые данные String в данные синтаксического анализа из любого источника: в памяти, в файле, в сети и т. Д. Теперь мы также можем тестировать parseпроще, поскольку нам не нужен файл на диске. проводить тесты против.


Иногда на самом деле есть два (или более) исключения, которые мы можем выбросить из метода, но если мы придерживаемся SRP и DIP, время, когда мы сталкиваемся с этой ситуацией, становится реже.

cbojar
источник
Я полностью согласен с упаковкой исключений нижнего уровня в соответствии с вашим примером. Мы делаем это регулярно и генерируем отклонения MyAppExceptions. Один из примеров, где я выкидываю несколько исключений, - при попытке обновить запись в базе данных. Этот метод генерирует RecordNotFoundException. Однако запись может быть обновлена, только если она находится в определенном состоянии, поэтому метод также создает исключение InvalidRecordStateException. Я думаю, что это действительно и предоставляет звонящему ценную информацию.
sdoca
@sdoca Если ваш updateметод настолько атомарен, насколько вы можете его создать, а исключения находятся на должном уровне абстракции, тогда да, похоже, вам нужно выбросить два разных типа исключений, поскольку есть два исключительных случая. Это должно быть мерой того, сколько исключений может быть выдано, а не этими (иногда произвольными) правилами линтера.
cbojar
2
Но если у меня есть метод, который читает данные из потока и анализирует их по ходу процесса, я не могу разбить эти две функции без чтения всего потока в буфер, что может быть громоздким. Кроме того, код, который решает, как правильно обрабатывать исключения, может быть отделен от кода, который выполняет чтение и анализ. Когда я пишу код для чтения и анализа, я не знаю, как код, вызывающий мой код, может обрабатывать исключения любого типа, поэтому мне нужно пропустить их обоих.
user3294068
+1: мне очень нравится этот ответ, особенно потому, что он решает проблему с точки зрения моделирования. Часто нет необходимости использовать еще одну идиому (например catch (IOException | SQLException ex)), потому что настоящая проблема заключается в модели / дизайне программы.
Джорджио
3

Я помню, как немного возился с этим, когда играл в Java некоторое время назад, но я не осознавал различия между проверенным и непроверенным, пока не прочитал ваш вопрос. Я нашел эту статью в Google довольно быстро, и она вызывает некоторые очевидные противоречия:

http://tutorials.jenkov.com/java-exception-handling/checked-or-unchecked-exceptions.html

Тем не менее, одна из проблем, о которых этот парень упоминал с проверенными исключениями, заключается в том, что (и я лично столкнулся с этим с самого начала с Java), если вы продолжите добавлять кучу проверенных исключений к throwsпредложениям в объявлениях ваших методов, а не только вам нужно добавить гораздо больше стандартного кода для его поддержки при переходе к методам более высокого уровня, но это также создает еще большую неразбериху и нарушает совместимость, когда вы пытаетесь представить больше типов исключений для методов более низкого уровня. Если вы добавляете проверенный тип исключения в метод более низкого уровня, то вам придется снова выполнить код и настроить также некоторые другие объявления методов.

Одна из причин смягчения, упомянутых в статье - и автору лично это не понравилось, - это создать исключение базового класса, ограничить ваши throwsпредложения только его использованием, а затем просто поднять его подклассы внутри себя. Таким образом, вы можете создавать новые проверенные типы исключений без необходимости повторного выполнения всего кода.

Автору этой статьи, возможно, это не очень понравилось, но в моем личном опыте это имеет смысл (особенно если вы в любом случае можете посмотреть, что представляют собой все подклассы), и я уверен, что именно поэтому вам дают совет ограничить все одним проверенным типом исключения. Более того, упомянутый вами совет фактически допускает использование нескольких проверенных типов исключений в закрытых методах, что имеет смысл, если это является их мотивом (или даже иным образом). В любом случае, если это просто приватный метод или что-то подобное, вы не пройдете половину своей базы кода, когда измените одну маленькую вещь.

Вы в основном спрашивали, является ли это общепринятым стандартом, но между вашим исследованием, которое вы упомянули, этой достаточно продуманной статьей, и просто рассказом из личного опыта программирования, оно, безусловно, никак не выделяется.

Panzercrisis
источник
2
Почему бы просто не объявить броски Throwableи покончить с этим, вместо того, чтобы придумывать всю свою собственную иерархию?
Дедупликатор
@Deduplicator По этой причине автору тоже не понравилась идея; он просто подумал, что вы могли бы использовать все непроверенные, если вы собираетесь это сделать. Но если у того, кто использует API (возможно, у вашего коллеги), есть список всех подклассовых исключений, которые происходят из вашего базового класса, я могу увидеть небольшую выгоду, по крайней мере, давая им понять, что все ожидаемые исключения в пределах определенного набора подклассов; тогда, если они чувствуют, что один из них более «управляем», чем другие, они не будут так склонны отказываться от специфического для него обработчика.
Panzercrisis
Причина, по которой проверенные исключения в целом являются плохой кармой, проста: они вирусные, заражающие каждого пользователя. Указание преднамеренной максимально широкой спецификации - это просто способ сказать, что все исключения не проверены, и, таким образом, избежать беспорядка. Да, документирование того, что вы, возможно, захотите обработать, является хорошей идеей для документации : просто знание того, какие исключения могут исходить от функции, имеет строго ограниченную ценность (не считая ни одного вообще / возможно, одного, но в любом случае Java не позволяет этого) ,
Дедупликатор
1
@Deduplicator Я не поддерживаю идею, и я не обязательно одобряю это также. Я просто говорю о направлении, откуда мог быть дан совет ОП.
Panzercrisis
1
Спасибо за ссылку. Это была отличная отправная точка для чтения по этой теме.
sdoca
-1

Бросать несколько проверенных исключений имеет смысл, когда есть несколько разумных действий.

Например, допустим, у вас есть метод

public void doSomething(Credentials cred, Work work) 
    throws CredentialsRequiredException, TryAgainLaterException{...}

это нарушает правило исключения, но имеет смысл.

К сожалению, обычно происходит то, что

void doSomething() 
    throws IOException, JAXBException,SQLException,MyException {...}

Здесь у вызывающего абонента мало шансов сделать что-то конкретное в зависимости от типа исключения. Поэтому, если мы хотим подтолкнуть его к осознанию того, что эти методы МОГУТ и иногда БУДУТ работать неправильно, выбрасывание исключения SomethingMightGoWrongExo достаточно и лучше.

Следовательно, правило не более одного проверенного исключения.

Но если в вашем проекте используется дизайн, в котором имеется несколько полностью проверенных исключений, это правило не должно применяться.

Sidenote: Что-то на самом деле может пойти не так почти везде, так что можно подумать об использовании? расширяет RuntimeException, но есть разница между «мы все делаем ошибки» и «это говорит о внешней системе, и иногда она будет недоступна, разберитесь с ней».

user470365
источник
1
«множественные разумные действия» вряд ли соответствуют srp - этот момент был изложен в предыдущем ответе
комнат
Оно делает. Вы можете обнаружить одну проблему в одной функции (обработка одного аспекта) и другую проблему в другой (также обработку одного аспекта), вызванную из одной функции, обрабатывающей одну вещь, а именно вызов этих двух функций. И вызывающий абонент может в одном слое try перехватить (в одной функции) ОБРАЩАТЬСЯ к одной проблеме, передать вверх другую и обработать ее в одиночку.
user470365