Выкидываете исключения для контроля потока - запаха кода?

Question

Рассмотрим этот код (в частности, Java):

public int doSomething()
{
    doA();

    try {
        doB();
    } catch (MyException e) {
        return ERROR;
    }

    doC();
    return SUCCESS;
}

Где doB() определяется как:

private void doB() throws MyException

По существу, MyException существует только в том случае, если doB() удовлетворяет некоторому условию (которое не является катастрофическим, но должно каким-то образом повышать это условие), так что doSomething() будет знать, что выйти с ошибкой.

Считаете ли вы приемлемым использование исключения, в данном случае для управления потоком? Или это запах кода? Если да, то как бы вы перестроили это?

Answer 1

Действительно ли важно, чтобы doC () выполнялся при сбое doB ()? Если нет, то почему бы просто не позволить исключению распространяться вверх по стеку, где оно может быть эффективно обработано. Лично я считаю использование кодов ошибок запахом кода.

Редактировать: В своем комментарии вы точно описали сценарий, в котором вы должны просто объявить

public void doSomething() throws MyException

Answer 2

Это полностью зависит от того, что это за ошибка, и какова работа метода. Если возвращение ERROR является допустимым способом обработки этой ошибки для вызывающей функции, почему это будет плохо?

Часто, однако, это является запахом. Учтите это:

bool isDouble(string someString) {
    try {
        double d = Convert.ParseInt32(someString);
    } catch(FormatException e) {
        return false;
    }
    return true;
}

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

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

bool isDouble(string someString) {
    bool success;
    Convert.TryParseInt32(someString, ref success);
    return success;
}

У такого рода исключений есть специальное имя, придуманное каким-то чуваком, чей блог я недавно читал. Но, к сожалению, я забыл его имя. Пожалуйста, прокомментируйте, если вы это знаете. И последнее, но не менее важное: приведенный выше псевдокод. Я не разработчик на C #, поэтому вышеприведенное не компилируется, я уверен, но TryParseInt32 / ParseInt32 демонстрирует, что я думаю, так что я собираюсь использовать C #.

Теперь к вашему коду. Давайте проверим две функции. Один пахнет, а другой нет:

1. Запах

public int setupSystem() {
    doA();

    try { doB(); }
    catch (MyException e)
    { return ERROR; } 

    doC();
    return SUCCESS;
}

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

2. Ok

public int pingWorkstation() {
    doA();

    try { doB(); }
    catch (MyException e)
    { return ERROR; } 

    doC();
    return SUCCESS;
}

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

Answer 3

Моя единственная проблема с кодом OP состоит в том, что вы смешиваете парадигмы - doB выдает ошибку, выдавая исключение, а doSomething показывает ошибку, возвращая код. В идеале вы бы выбрали один или другой. Конечно, в устаревшем обслуживании у вас может не быть такой роскоши.

Если вы выберете возврат кодов ошибок, это нормально, но мне это не нравится, потому что он побуждает вас использовать побочные каналы (например, глобальные переменные) для передачи состояния при сбое, а не связывать это состояние в исключение и позволять ему пузыри стека, пока вы не можете что-то сделать с этим.

Answer 4

Мне никогда не нравилось использовать исключения для потока управления (в некоторых языках, таких как CLR, они дороги).

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

public int doSomething()
{
    doA();

    if (!doB()) {
        return ERROR;
    }

    doC();
    return SUCCESS;
}

Answer 5

Исключения следует использовать, когда:

• функция не может завершиться нормально, а
• когда нет возвращаемого значения, которое можно использовать для указания сбоя, или
• когда выброшенное исключение сообщает больше информации, чем return FAILURE; может, например, вложенные исключения или тому подобное.

Прежде всего, помните, что исключения, такие как возвращаемые значения или параметры метода, являются просто сообщениями, отправляемыми между различными частями кода. Стремитесь оптимизировать баланс между информацией, передаваемой этими методами, и простотой API. Когда простой флаг SUCCESS / FAILURE - это все, что нужно (и метод не должен возвращать другие значения), используйте это. Если метод уже должен возвращать значение, вам обычно нужно использовать исключение (которое, с одной стороны, просто «исключительное» возвращаемое значение из метода). Если информация об ошибке, которая должна быть передана, слишком обширна, чтобы передать ее в возвращаемом значении (например, причина сбоя ввода-вывода), используйте исключение.

Наконец, обработка ошибок - это проектное решение, и не существует единого набора правил, который бы подходил всем, чтобы охватить все случаи.

Answer 6

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

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

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

В качестве примера того, что не нужно сделать, это будет использовать исключения для управления потоком:

public int calculateArraySize(Object[] array)
{
   int i = 0;
   try
   {
      array[i++];
   }
   catch (ArrayIndexOutOfBoundsException ignore) {}
   return i;
}

Я верю, что это вернет правильный результат, но он будет ужасно медленным и неэффективным, а также трудным для чтения людьми, привыкшими к исключениям, используемым должным образом. ; -)

С другой стороны, что-то вроде этого было бы хорошо по моему мнению:

public void myMethod(int count, Foobar foo) throws MyPackageException
{
   validateArgs(count, foo);

   // Stuff
}

private void validateArgs(int count, Foobar foo) throws MyPackageException
{
   if (m_isClosed)
   {
      throw new IllegalStateException("Cannot call methods on a closed widget");
   }

   if (count < 0)
   {
      throw new IllegalArgumentException("count must be greater than zero");
   }

   foo.normalise(); // throws MyPackageException if foo is not fully initialised at this point
}

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

Answer 7

Исключения не должны использоваться только для управления потоком. Проверенные исключения должны использоваться, чтобы сообщить вызывающему коду, что определенные условия контракта API не были выполнены. Я бы не стал проектировать doSomething для обработки случаев, когда вызовы к doB часто бывали неудачными с использованием блока try/catch. Если бы вам приходилось иметь дело с частыми неудачами, я бы разработал doB для возврата успеха или неудачи boolean, чтобы указать его вызывающим методам, продолжать ли их собственные методы doC -типа: *

public int doSomething() {
    doA();

    if ( doB() )
       doC();
       return SUCCESS;
    } else { 
       return ERROR; 
    }
}    

Answer 8

Можно генерировать исключение в случае ошибки, как в doB (). Но проблема в функции doSomething ().

Вы не должны использовать операторы return для индикации успеха или неудачи. Вы должны сделать:

try { 
    doB();
} catch (MyException e) {
    throw MyException2(e);
}

Answer 9

Исключения - для нестандартного поведения. Так что да можно использовать их для управления потоком.

Как говорит Солнце:

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

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

Только не забывайте не создавать подкласс RuntimeException, а Exception, поскольку они быстрее.

Answer 10

В зависимости от логики doB у вас могут быть некоторые возвращаемые значения, указывающие, нормально ли это, или нет, а затем doSomething может использовать возвращаемое значение, чтобы соответствующим образом обработать ситуацию.