Удаление лишних блоков try-catch - PullRequest
Новая
       2

Удаление лишних блоков try-catch

9 голосов
/ 10 августа 2011

Я занимаюсь рефакторингом WinForms-приложения среднего размера, написанного другими разработчиками, и почти каждый метод каждого класса окружен блоком try-catch. В 99% случаев эти блоки захвата только регистрируют исключения или ресурсы очистки и возвращают статус ошибки.

Я думаю, очевидно, что в этом приложении отсутствует надлежащий механизм обработки исключений, и я планирую удалить большинство блоков try-catch.

Есть ли минус в этом? Как бы вы это сделали? Я планирую:

  • Для надлежащей регистрации исключений и предотвращения их распространения пользователю предоставляется обработчик Application.ThreadException

  • В тех случаях, когда существует ресурс, требующий очистки, оставьте блок try-catch таким, как он есть

Обновление : лучше использовать блоки using или try-finally. Спасибо за ответы.

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

Любые исправления / предложения приветствуются.

Редактировать: В 3-м пункте под "return-false-on-error" я имел в виду такие методы: 

bool MethodThatDoesSomething() {
    try {
       DoSomething(); // might throw IOException
    } catch(Exception e) {
       return false;
    }
}

Я хотел бы переписать это как: 

void MethodThatDoesSomething() {
   DoSomething(); // might throw IOException
}

// try-catch in the caller instead of checking MethodThatDoesSomething's return value
try {
   MethodThatDoesSomething()
} catch(IOException e) {
   HandleException(e);
}

Ответы [ 7 ]

2 голосов
/ 10 августа 2011

Для очистки лучше использовать try-finally или внедрить IDisposable, как предложено Amittai.Для методов, которые возвращают bool при ошибке, попробуйте вернуть false, если условие не выполняется.Пример. 

bool ReturnFalseExample() {
    try {
        if (1 == 2) thow new InvalidArgumentException("1");
    }catch(Exception e) {
       //Log exception  
       return false;
    }

Скорее измените это. 

bool ReturnFalseExample() {
    if (1 == 2) {
       //Log 1 != 2
       return false;
    }

Если я не ошибаюсь try catches - дорогостоящий процесс и, если возможно, вы должны попытаться определить, не выполнено ли условиеа не просто ловить исключения.} * +1010 *

2 голосов
/ 10 августа 2011

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

Сможете ли вы тогда сообщить пользователю, что произошло?Будут ли там все исключения?

"В тех случаях, когда существует ресурс, требующий очистки, оставьте блок try-catch как"

. Вы также можете использовать try-finally блоки, еслиВы хотите, чтобы исключение было обработано в другом месте.Также рассмотрите возможность использования ключевого слова using в ресурсах IDisposable.

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

Это зависит от метода.Исключения должны возникать только в исключительных ситуациях.FileNotFoundException просто странно для метода FileExists(), но совершенно законно быть брошенным OpenFile().

1 голос
/ 10 августа 2011

Я думаю, что ваша стратегия удаления блока try / catch, который, как кажется, выполняет обычную бездумную регистрацию, в порядке.Очевидно, что оставить код очистки необходимо.Тем не менее, я думаю, что для вашего третьего пункта необходимы дополнительные разъяснения.

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

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

1 голос
/ 10 августа 2011

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

Хороший подход состоит в том, чтобы сначала зарегистрировать исключение, а затем Перезапустить ваше приложение, точно так же, как это делала Microsoft при сбое офиса или Visual Studio. Для этого вам необходимо обработать неиспользуемое исключение домена приложения, поэтому: 

AppDomain.CurrentDomain.UnhandledException += OnCurrentDomainUnhandledException;

//Add these two lines if you are using winforms
Application.ThreadException += OnApplicationThreadException;
Application.SetUnhandledExceptionMode(UnhandledExceptionMode.CatchException);

private void OnCurrentDomainUnhandledException(object sender, System.Threading.ThreadExceptionEventArgs e)
{
    //Log error

    //RestartTheApplication
}

Здесь пример того, как перезапустить ваше приложение.

1 голос
/ 10 августа 2011

В качестве опции для «return-false-on-error» вы можете очистить код следующим образом: 

    static class ErrorsHelper {
        public static bool ErrorToBool(Action action) {
            try {
                action();
                return true;
            } catch (Exception ex) {
                LogException(ex);

                return false;
            }
        }

        private static void LogException(Exception ex) {
            throw new NotImplementedException();
        }
    }

и пример использования: 

    static void Main(string[] args) {
        if (!ErrorToBool(Method)) {
            Console.WriteLine("failed");
        } else if (!ErrorToBool(() => Method2(2))) {
            Console.WriteLine("failed");
        }
    }

    static void Method() {}

    static void Method2(int agr) {}
1 голос
/ 10 августа 2011

Вы можете попытаться использовать AOP.

Например, в AOP через PostSharp вы можете обрабатывать исключения в одном центральном месте (фрагмент кода) как аспект.

Посмотрите напримеры в документации, чтобы иметь представление => Документы по обработке исключений с PostSharp .

0 голосов
/ 10 августа 2011

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

...