Дополнительный оператор попытки в операторе выгоды - кодирует запах?

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

const int MAX_RETRY = 3;

public static void DoWork()
{
    //Do Something
}

public static void DoWorkWithRetry()
{
    var @try = 0;
retry:
    try
    {
        DoWork();
    }
    catch (Exception)
    {
        @try++;
        if (@try < MAX_RETRY)
            goto retry;
        throw;
    }
}

Другой способ - сгладить try / catch блоки, полезные, если вы используете какой-то API, поддерживающий исключения:

public void Foo()
{
  try
  {
    HelperMethod("value 1");
    return; // finished
  }
  catch (Exception e)
  {
     // possibly log exception
  }

  try
  {
    HelperMethod("value 2");
    return; // finished
  }
  catch (Exception e)
  {
     // possibly log exception
  }

  // ... more here if needed
}

В этом случае вы знаете, что это «исключение», вероятно, произойдет, поэтому я бы предпочел простой подход - оставить исключения для неизвестных событий.

//run initial process
try
{
    //initial information used in helper method
    string s1 = "value 1";

    //call helper method
    if(!HelperMethod(s1))
    {
        //backup information if first process generates an exception in the helper method
        string s2 = "value 2";
        if(!HelperMethod(s2))
        {
          return ErrorOfSomeKind;
        }
    }
    return Ok;
}
catch(ApplicationException ex)
{
    throw;
}

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

Например, в среде, где вы используете зеркалирование SQL Server, возможно, что сервер, к которому вы подключены, перестанет быть главным промежуточным соединением.

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

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

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

Если вам нужно повторить попытку более одного раза, подумайте о том, чтобы установить задержку повтора «автоматическая отсрочка» - первая ошибка повторяется немедленно, вторая после задержки (скажем) в 1 секунду, затем удвоился до (скажем) 90 секунд. Это должно помочь предотвратить перегрузку ресурсов.

Я бы также предложил реструктурировать ваш метод, чтобы у вас не было внутренних попыток / ловушек.

Например:

bool helper_success = false; 
bool automatic_retry = false; 
//run initial process
try
{
    //call helper method
    HelperMethod();
    helper_success = true; 
}
catch(Exception e)
{
    // check if e is a transient exception. If so, set automatic_retry = true 
} 


if (automatic_retry)
{    //try catch statement for second process.
    try
    {
        HelperMethod();
    }
    catch(Exception e)
    {
        throw;
    }
}

Вы можете эмулировать сигнатуры метода TryParse в C #:

class Program
{
    static void Main(string[] args)
    {
        Exception ex;
        Console.WriteLine("trying 'ex'");
        if (TryHelper("ex", out ex))
        {
            Console.WriteLine("'ex' worked");
        }
        else
        {
            Console.WriteLine("'ex' failed: " + ex.Message);
            Console.WriteLine("trying 'test'");
            if (TryHelper("test", out ex))
            {
                Console.WriteLine("'test' worked");
            }
            else
            {
                Console.WriteLine("'test' failed: " + ex.Message);
                throw ex;
            }
        }
    }

    private static bool TryHelper(string s, out Exception result)
    {
        try
        {
            HelperMethod(s);
            result = null;
            return true;
        }
        catch (Exception ex)
        {
            // log here to preserve stack trace
            result = ex;
            return false;
        }
    }

    private static void HelperMethod(string s)
    {
        if (s.Equals("ex"))
        {
            throw new Exception("s can be anything except 'ex'");
        }
    }

}

Вот еще один шаблон:

// set up state for first attempt
if(!HelperMethod(false)) {
        // set up state for second attempt
        HelperMethod(true);
        // no need to try catch since you're just throwing anyway
}

Здесь HelperMethod равно

bool HelperMethod(bool throwOnFailure)

, а возвращаемое значение указывает, произошел ли успех (т. Е. false указывает на сбой, а истина указывает на успех). Вы также можете сделать следующее:

// could wrap in try/catch
HelperMethod(2, stateChanger);

, где HelperMethod равно

void HelperMethod(int numberOfTries, StateChanger[] stateChanger)

, где numberOfTries указывает количество попыток перед генерацией исключения, а StateChanger [] - массив делегатов, которые будут изменять состояние для вас между вызовами (например, stateChanger [0] вызывается перед первой попыткой, stateChanger [1] вызывается перед второй попыткой, и т. д.)

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

Конечно, вам нужно перехватить более конкретное исключение, чем вы сейчас (не перехватывайте исключение базового класса !) и вы должны просто throw вместо throw e , если вам нужно повторно выбросить исключение после регистрации, очистки и т. д.

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

Все должно быть не так уж плохо. Просто четко задокументируйте, почему вы это делаете, и ОБЯЗАТЕЛЬНО попытайтесь отловить более конкретный тип исключения.

Да, более общий шаблон включает в себя базовый метод перегрузка, которая принимает параметр int try , а затем условно вызывает себя рекурсивно.

   private void MyMethod (parameterList)
   {  MyMethod(ParameterList, 0)l }

   private void MyMethod(ParameterList, int attempt)
   {
      try { HelperMethod(); }
      catch(SomeSpecificException)
      {
          if (attempt < MAXATTEMPTS)
              MyMethod(ParameterList, ++attempt);
          else throw;
      }
   }

Вообще говоря, это не проблема, и я не знаю запаха кода.

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

Общая идея помещения try-catch внутри catch родительской try -catch мне не кажется запахом кода. Я могу думать о других законных причинах для этого - например, при очистке операции, которая завершилась неудачно, когда вы не хотите, чтобы когда-либо выдавалась другая ошибка (например, если операция очистки также не удалась). Однако ваша реализация вызывает у меня два вопроса: 1) Комментарий Вима, и 2) вы действительно хотите полностью игнорировать причину сбоя операции (исключение e1)? Независимо от того, успешно или нет второй процесс, ваш код ничего не делает с исходным исключением.

Я бы не сказал, что это плохо , хотя я почти наверняка реорганизовал бы второй блок кода во второй метод, так что пусть он будет понятен. И, вероятно, поймать что-то более конкретное, чем Exception . Иногда требуется вторая попытка, особенно для таких вещей, как реализации Dispose () , которые сами могут вызывать (WCF, я смотрю на вас).

Было бы немного яснее, если бы вы вызывали другую функцию в catch, чтобы читатель не подумал, что вы просто пытаетесь повторить ту же функцию снова и снова. Если происходит состояние, которое не показано в вашем примере, вы должны как минимум тщательно его задокументировать.

Вы также не должны throw e2; таким образом: вы должны просто throw; если собираетесь работать с исключением, которое вы вообще поймали. Если нет, вам не следует пробовать / catch.

Если вы не ссылаетесь на e1, вам следует просто catch (Exception) или еще лучше catch (YourSpecificException)

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

Вам нужно всего лишь:

try
{
    //call helper method
    HelperMethod();
}    
catch(Exception e1)
{
    // maybe log e1, it is getting lost here
    HelperMethod();
}

Я знаю, что недавно выполнил вышеупомянутую вложенную попытку catch для обработки данных декодирования, когда две сторонние библиотеки выдают исключения при сбое декодирования (попробуйте декодировать json, затем попробуйте декодировать base64), но я предпочитаю иметь функции возвращают значение, которое можно проверить.

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

Если функция не может предоставить осмысленный ответ, это обычно не является фатальной проблемой (в отличие от неверных входных данных).

Похоже, что основной риск при использовании вложенного try catch заключается в том, что вы также перехватываете все другие (возможно, важные) исключения, которые могут произойти.