Pull to refresh

Comments 68

Неплохая старт-статья. В принципе, всё так, хотя и не заметил чего-то нового для себя.

Правда, я совсем не понял того, что написано про рефакторинг. Может стоит также изобразить на примере?
Изначально рефакторинг писался как перекрытие спины: если что — всегда можно перейти к коду без исключений. 11 пунктов писал с большой не охотой — потому что сам только мельком бы просмотрел. Мой основной аргумент: примеры для рефакторингов не полные и громоздкие. Но вы второй задавший этот вопрос (первый online), а потому сомнения исчезли — пример написан. Как только пойму, как не выводя статью из обсуждения ее исправить, так сразу.
Надо новый термин ввести — throwФобия.
Примеров кода действительно не хватает, описание рефакторинга не очень читаемое.
throwФобия

Вообще часто чем меньше исключений и чем более они локазилованы — тем лучше.
Пример — можно написать класс, выполняющий какие либо вычисления и принимающего в конструктор путь к файлу/поток для записи результата.
А можно сделать так, чтобы этот класс возвращал объект результата, и написать форматтер, который будет этот объект результата писать в поток.
Во втором случае все исключения, завязанные на доступ к внешнему ресурсу для сохранения — выделяются в отдельный класс, и стабильность системы повышается.
У вас речь о разделении обязанностей. Какое это имеет отношение к исключениям?
Прямое отношение, исключения должны выбрасываться по возможности только там, где на пути по стеку они не уничтожат работу, которая не зависит от причины возникновения исключения.
Простите, что? То есть метод, выбрасывающий исключения, должен внутри себя предполагать, как выглядит стек трейс до входа в него? Для публичных методов это сильно пахнет нарушением инкапсуляции.

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

Например в Java компилятор может проверить, что произведён перехват с помощью catch, но не может проверить, его корректность или наличие корректного finally. В С# ситуация такая же AFAIK.
Мне сразу вспомнились холиварные статьи о goto. Здесь точно так же не хватает реальных примеров, где throw делает жизнь заметно легче.

Лично я не считаю, что в процессе нормальной работы программы должны возникать какие-либо исключения. В конце концов — подумайте о коллегах. Вы сделаете отладку кошмаром. Даже если отключить break on exceptions, debug output будет захламлён, и полезная информация исчезнет из виду.

Exceptions, Исключения — на то они так и называются. Возникают в исключительных ситуациях.
foreach(string element in elements)
{
    try
    {
        int someData = int.Parse(element);
        result.Add(someData);
    }
    catch
    {
    }
}

Не совсем throw-филия, но вполне себе exception-филия.
Для этого есть TryParse
foreach (var element in elements)
{
int someData;
if (int.TryParse(element, out someData))
result.Add(someData);
}


Можно в две строчки, но неэффективно:
int i;
var result1 = elements.Where(e => int.TryParse(e, out i)).Select(e => int.Parse(e));

Жаль, что нету метода int? Parse(s), который возвращает null в случае неудачи. Но что-то я увлёкся.
Так я это и имею ввиду, что надо писать TryParse, а вышеприведенный мной код — пример throw(exception)-филии.
Если уж писать в одну строчку — то лучше сделать для этого заранее Extension:
     var result = DataFilter.Filter<string, int>(sourceData, int.TryParse);

    public delegate bool TryParse<in TInput, TOutput>(TInput input, out TOutput output);

    public static class DataFilter
    {
        public static IEnumerable<TResult> Filter<TSource, TResult>(IEnumerable<TSource> sourceSequence,
                                                                    TryParse<TSource, TResult> tryParseFunction)
        {
            List<TResult> result = new List<TResult>();
            foreach (var sourceElement in sourceSequence)
            {
                TResult resultElement;
                if (tryParseFunction(sourceElement, out resultElement))
                {
                    result.Add(resultElement);
                }
            }

            return result;
        }
    }
Тьфу, Helper заранее сделать, поздно уже.
А я бы таки сделал тот самый extension метод с Nullable:

var result = elements.Select(e => e.ToInt()).Where(e => e != null);

public static class StringExtensions
{
public static int? ToInt(this string s)
{
int i;
return int.TryParse(s, out i) ? i : (int?) null;
}
}



Более лаконично, и ленивое выполнение.
Тогда придется писать каждый раз новый экстеншн на каждый новый тип, в моем примере все что требуется — соблюдать сигнатуру метода TryParse, которую обычно везде соблюдают.
Ну и потом — много extension'ов — сильное захламление IntelliSense.
Ты собираешься парсить строку дважды??

я бы так сделал:

int i;
var result1 = elements.Where(e => int.TryParse(e, out i)).Select(e => i)

ну и можно замкнуть перечисление:

int i;
var result1 = elements.Where(e => int.TryParse(e, out i)).Select(e => i).ToList()
Во-первых, я отметил, что это неэффективно.
Во-вторых, твой код даже не скомпилируется. Недавно пользуешься linq и лямбдами?
Аха, давно. Вот так скомпилируется:

int i=0;
s.Where(e => int.TryParse(e, out i)).Select(e => i);
Но если он есть и подходит для логики «cancel» — глупо его не использовать.

Если у меня есть дробовик, и он подходит для устранения очереди в банке — глупо его не использовать! Что может случиться плохого? Приедут дяди с мигалками и объяснят что так делать не хорошо, устранив вас из общества на много лет.

Подобно этому, если вы начнете вот так писать в крупном проекте, то придут злые дяди и устранят вас из этого проекта (а может и из компании) на много лет. И сидите там потом и хоть обдоказывайтесь и обспорьтесь о том, что дробовик — идеальное средство разгона очередей, потому что вместо какой-то там возни бабах — и все убежали. А то, что пара трупов? Да кого это волнует!
Политика проверки по флагу:
— Дорогой, тебе нравится туалетная вода, которую я тебе купила?
— Нет.
— Ок, я сейчас что-нибудь придумаю.

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

Политика исключений:
— Дорогой, я купила тебе туалетную воду, пользуйся.
*хватаясь за ремень* Ах ты ж стерва, я терпеть не могу такой аромат!

Как надо было сделать:
— Дорогой, какую туалетную воду ты любишь?
— %result%.
— Дорогой, я купила тебе туалетную воду %result%, пользуйся.


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

«Как надо было сделать»:
— Дорогой, какую туалетную воду ты любишь?
— %result%.
… Прошло время…
— Дорогой, я купила тебе туалетную воду %result%, пользуйся.
— Да меня тошнит уже от неё
— О_о
Ну не совсем согласен. Все же обычно выкидывание исключения задевает и ту работу, которая могла не зависит от исключения. Откат к предыдущему стабильному состоянию как раз и подразумевает такое поведение. Ну а побритый кот — это например неосвобожденные блокировки и прочие вещи, которые находятся в некорректном состоянии из-за ошибки.
Для гарантированного освобождения ресурсов существуют специальные языковые конструкции: finaly, using. Возврат к стабильному состояния с их помощью внушает куда больше доверия, чем операции по else. А потом, если мы говорим об высвобождении ресурсов, их необходимо использовать в любом случае — ThreadAbortException или OutOfMemoryException ни кто не отменял. Если же писать и там и там — получим дублирование.
И вы, безусловно, можете гарантировать на этапе написания кода, что в нем не существует ни одного места, где из-за ошибки теряется блокировка или остается объект с поврежденным состоянием?
Использование исключений только там, где они действительно нужны, а не где они — один из способов — позволяет упростить программу, и следовательно — избежать ошибок (если только вы не научились программировать без ошибок с первого раза).

// В случае OutOfMemory обычно проще сохранить все что можно сохранить из текущего состояние программы и выйти.
Конечно могу.
Если я пишу:

using(new FileInfo("1.txt").OpenText())
  throw new MyException();


Я гарантировано освобождаю блокировку файла. Если нет — язык в топку.
Восстановление же фрагментов состояния в разных местах, на мой взгляд, разновидность запаха стрельба дробью (термин по Фаулеру).
Как же вы недальновидно пишете, как будто весь код — это те две строчки, которые вы написали, в сторонних библиотеках нет никаких ошибок, в другом коде тоже нет никаких ошибок.
Так может вы и IDE не пользуетесь? С такими навыками «конечно могу» можно и в блокноте без ошибок писать. Я бы тоже хотел так, но к сожалению в реальном мире это невозможно.

Ваш же пример и содержит ошибку, вы же должны знать, что using(statement) в C# в вашем случае эквивалентно:
var textStream = new FileInfo("1.txt").OpenText();
try
{
    throw new MyException();
}
finally
{
    if (textStream != null)
    {
        ((IDisposable)textStream).Dispose();
    }
}
Что делает его бесполезным. А в общем случае эксепшн может вылетить и до начала using.
Знаю, использую, не пишу.

И что это меняет.
Мы насколько я помню говорим о восстановлении состояния (и как частности: освобождение блокировок). А не о бескрайних полях синтаксического сахара C# и проверки результатов инициализации состояния.
По поводу двух строк: я лишь предложил крайний случай закона Мерфи в минимальной реализации. Вы внимательно не анализировали их, а поняли меньше чем за секунду. Следовательно, цели с которой они написаны, достигнуты.
Если же throw new… заменить на вызов «сторонних библиотек» как вы по сути предлагаете, то защищаемый мной подход даст гораздо больше гарантии не потерять стабильное состояние системы.
С помощью if можно проверить результат на null-value, а предполагая возможность exception-ов можно реализовать откат работы, если точно не сказано что все OK — паттерн транзакция (http://insidecpp.ru/patterns/transaction/).
Ну во первых далеко не всегда возможно корректное освобождение состояния, т.к. может остаться висеть ссылка на объект, который что-то держит. В менее тривиальных ситуациях finally просто не получится написать, т.к. контекст не ограничен одним методом.
Ну и все же я говорил к тому, что использование исключений надо минимизировать, а именно — чтобы через них не была реализована логика, чтобы исключение было ошибкой.
StreamReader textStream = null;
try
{
    textStream = new FileInfo("1.txt").OpenText()
    throw new MyException();
}
finally
{
    if (textStream != null)
        ((IDisposable)textStream).Dispose();
}
К чему этот комментарий? Мой ответ был о том, как разворачивается using, а как писать правильно этот код я знаю.
А вообще даже этот код в некоторых случаях не освобождает блокировку файла, в случае таких опасных исключений как ThreadAbortException, или, например OutOfMemoryException. И если в последнем случае вообще надо бы быстрее закрывать программу сохраняя что есть, то если кто-то додумается прервать ваш поток, без проблем не отделаетесь.
Сорри, я думал разворачивается, как я написал, по крайней мере почему-то так отложилось. Проверил сейчас IL код, и действительно, у вас и в MSDN правильно написано ;-)
Блин, при всём уважении к автору, прошу его: поправьте орфографию!
Не правда. Они считают, что в их конкретной ситуации (уже куча кода написана без использования исключений) это неоправданно — об этом пишут в обсуждении темы.
> файл закончился (reader поднимает исключение)
В данном случае обычной практикой является возвратить значение -1, если символ не найден:

int FindSymbol(TextReader reader, char symb)

И вот почему:

for ( int i = findSymbol( r, 'a' ); i >= 0; i-- ) {
doSomethingWithTheseSymbols( i );
}

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

P.S. тут недавно проходил обзор языка Go, и того извращения, которое они придумали вместо эксепшнов…
Самое неприятное при использовании исключений — это когда их забывают ловить. А ловить исключение, брошенное из другого потока — весёлое занятие. Ещё можно не ловить бросаемые исключения в конструкторах…
Клиенту больше не надо анализировать сложный результат, он попросил найти текст — ему нашли; если нет — он об этом даже не узнает.

Конечно, анализировать ошибки это для неудачников. У реальных пацанов всё всегда OK… ну если не OK то по крайней мере хватает стали в яйцах сказать пользователю, что OK.
Где-то код анализа проблеммы находиться обязан. Но это место не обязано распологаться непосредственно под вызовом критического кода. Это приводит к размытию основоной идеи кода.
Что делает if(-1==reader.ReadByte()) — он пытается интерпритировать ответ. Зачем интерпретировать если можно получить точный, структурированный, расширяемый результат от первого лица?
В то же время ветка if, отвечающая за обработку нестандартных ситуаций в большинстве случаев заканчивается return null-Value (или аналогично — например break). Тем самым мы делегируем ответственность принятия решения о дальнейших действиях.
Оба механизма уже реаизованы: тип исключения точно описывает проблему, а просачивание позволяет определить место где действительно достаточо данных о принятии однозначного решения.
Он должен находиться не «где-то» а прямо тут. Потому-что прямо тут можно забыть закрыть открытый коннект к БД или обратиться к неинициализированной переменной с вываливанием другого исключения, превращающего поиск источника ошибки в детективный роман.

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

Использование исключений упирается в талант и прозорливость архитектора системы. Они даже в теории ограничены ограничены. А на практике обычно архитектора вообще нет.
UFO just landed and posted this here
UFO just landed and posted this here
30% это не так много. В любом случае факт просадки производительности из за Exception-ов должен определять profiler.

Сваливание тестов во время рефакторинга, говорит о большой величине шага в процессе его осуществления. На достаточно хорошо покрытой тестами системе правильней откатиться и уменьшить шаг.
А после: недостаточность набора тестов, по крайней мере проверяющих эти аспекты работы. Для теста не особо важно возвращает тестируемая логика null, ResWork.Fail или поднимает Exception — если существует проверка одного, то тест плавно превратится в другое.

Мы же об автотестах?
UFO just landed and posted this here
Честно говоря, не хотел вас обидеть словами «правильно» и «не правильно». В первой редакции даже написал (там где о шаге) «в описываемой вами» — то есть отдавал должное факту наличия автотестов.

Если не так все радужно, то здесь выход толко локализация шагов рефакторинга. И рефакторинг заменой кода на оттестированый. Сам мучуюсь.

30%: из-за них я месяц назад не стал рисковать. У меня было 2 и 3 дня. Мне показалась что это цифры одного порядка. Хотя конечно каждая система уникальна.
UFO just landed and posted this here
Этот подход очень неплохо показал себя в библиотеке boost.asio. Там для каждого действия введены 2 функции: одна с исключением, другая — без него. Для функции без исключения передается специальный параметр, который говорит о том, какая конкретно ошибка произошла. Таким образом можно использовать любой из подходов без потери производительности, а также их комбинировать. Но написание такой библиотеки — довольно хлопотное дело.
Эксепшены тормозят выполнение раз в 10. Я тоже не верил пока не нарвался.
К примеру откройте Visual Studio и выполните простой цикл, чтобы внутри него кидалось и обрабатывлось исключение.
Затем выполните этот же цикл без блока try/catch.

Лично у меня разница получилась в 10 раз, что там где есть исключение работает медленнее.

Я повторюсь, что в это не верится, пока на практике не столкнешься, насколько они работу замедляют.
Я этого не отрицаю. Сталкивался.

Но цикл с try catch внутри и обработка реальных данных — это не совсем одно и тоже. Повторюсь, факт просадки производительности может подтвердить только profiler. Только он может сказать что данный фрагмент выполняется достаточно часто, чтобы стать причиной проблем в работе системы вцелом.

Например, внутри цикла шифрования я бы этого делать не стал, а для извещения о браке в ключе вполне.
Я столкнулся в ситуацией, когда мне надо было обработать около 1000 объектов из базы, и создать новые объекты, в каждом из котором где то 20 полей, то есть получается 20000 полей надо было обработать. И решил не заморачиваться с проверками на null, а просто обернуть в try/catch и возвращать пустую строку в случае null. Имеется ввиду не просто object.Property, а например object.anotherObject.anotherProperty, где каждый элемент может быть null. Получилась огромная просадкда производительности. С тех пор я убедился что эксепшены надо юзать осторожно.
К моменту получения информации о существовании проблемы вы уже разработали работающий функционал. Разработка окончена (по крайней мере на этом локальном участке кода на данной итерации). Устранение непрерывного потока exception-ов является задачей процесса профилирования, которую в подавляющем большинстве случаев решать не приходится.
В С++, насколько я слышал и наблюдал сам, оптимизируется не случай, когда исключение выбрасывается, а случай, когда оно не выбрасывается (т.н. zero-cost exception handling). Т.е. разработчики компилятора считают, что вы будете кидаться исключениями только в редких нештатных ситуациях.

У себя тоже в «синтетическом» примере я наблюдал разницу где-то в 1-3%, которая в зависимости от поведения оптимизатора в компиляторе (без указания inline отказался автоматически инлайнить функцию с исключением) была в пользу исключений или в пользу возврата кода ошибки.
Как вы относитесь к маленьким функциям? Я вот считаю, что другие просто не имеют права на существование. При таком отношении периодически возникают ситуации, что без поднятия исключений просто не куда. Например, надо выйти из цикла расположенного выше по стеку вызовов. Не передавать же флаг завершения, в конце концов. Или передать…


Такое ощущение, что у вас единственный критерий декомпозиции кода на функции — это размер получаемых функций. В результате получается лапша-код с которым без бензопилы исключений не справиться. Ну вот пример: откуда функция знает, что её вызывают из цикла? Да какое вообще её дело из цикла ли её вызывают, или ещё откуда? Получаем: «Ты туда не ходи, ты сюда ходи, ексепшен башка упадет, совсем мертвый будешь».
Функция должна быть самодостаточна, иметь хороший идентификатор и сигнатуру, по которой программист может понять где и как этой функцией можно и нужно пользоваться даже без заглядывания в её код. Исключения же делают интерфейс функции неявным. Программисту теперь мало посмотреть на сигнатуру, ему еще надо изучить код, чтобы понять в каких случаях какие исключения выкидываются. Это не проблема когда код был написан недавно и программист помнит это наизусть, либо когда код пишется на выброс. Но вот дальнейшая поддержка сильно усложняется.
Анализируя типичный rant по поводу того, какие исключения плохие, я выяснил, что многие их просто не умеют готовить. Не понимают, в каких случаях это удобно и почему это лучше кодов возврата. Писать очередное объяснение лень, все уже давно написано. Судя по всему, дело в том, что у некоторых людей отсутствует часть мозга, отвечающая за исключения.
Еще один аспект не учли — отладка.
Если нужно в отладке оказаться на месте выполнения исключения по ошибке, а в проекте до этого в циклах 100500 раз вызываются исключения для бизнес-логики, то отладка станет мучением.
Если я правильно понял вопрос.

Если вы чем-то отделяете 100500-ое исключение от остальных 100499 вы должны об этом в коде явно сказать (сделать ответвление в иерархии наследования исключений). Ни кто же не утверждает что все исключения class-а Exception. Иначе это не чем не будет отличаться от возврата -1.

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

Другими словами это не проблема Exception-ов как таковых.
В последнем примере должно быть отрицание:
static class SrcClassHelper
{
  public static void Exec(this SrcClass src)
  {
    if (!src.TryExec()) // Здесь было без !
      throw new MyException();
  }
}
Например, в Java накладным расходом на throw является сохранение в объекте Exception всего stack trace, что являет довольно дорогой операцией. Поэтому такими средствами нужно пользоваться либо в случае некритичности по времени, либо при завершении вселго цикла обработки после обработки исключения. Если же ловить исключение на каждой итерации — просад может быть о-о-очень значительным.
Так что всему свое место, IMHO.
Я с вами абсолютно согласен.

Перехватывать Exception-ы надо только там, где ты точно знаешь как их обработать. Если же мы каждую функцию будем обрамлять в try catch, то это будем ни чем не лучше GetLastError, возврата -1, или макроса ASSERT в который, дядьки с большими головами, советовали обрамлять каждый вызов в C++ коде.
есть 2 вида реализации работы с исключениями:
— zero cost, по предгенеренным таблицам, большинство нынешних компиляторов по умолчанию использует этот вариант
— динамический
все performance issue исследованы и описаны в «Technical Report on C++ Performance»
public.research.att.com/~bs/performanceTR.pdf
можно прочитать и побороть свою throwФобию
Еще не забываем, что выкидывать исключение надо в действительно исключительной ситуации для обработки ошибок, а не просто ради быстрого выхода из кучи вложенностей
Мы обычно говорим «кидать», а не поднимать исключения.
Я как-то об этом не задумывался. Но однозначно существует оба термина. Скорее всего это переводы raise и throw, используемые в разных языках программирования.
Sign up to leave a comment.

Articles