Comments 55
UFO just landed and posted this here
В том то и дело, что не на новичков. Цель не осветить все рефакторинги с примерами. Это прекрасно сделано у Фаулера. Цель — напомнить, как важно следить за кодом, который вы пишете.
«Дамп потока сознания» сопровождать невозможно, а часто код выглядит именно так. Конечно, тот, у кого в системе методов больше сотни строк нет, все покрыто тестами, проводится ревью кода, и не поймет иронии :) А вот если вы знаете в своей системе с десяток методов длиннее тысячи строк, десяток классов размером больше пяти тысяч строк…
«Дамп потока сознания» сопровождать невозможно, а часто код выглядит именно так. Конечно, тот, у кого в системе методов больше сотни строк нет, все покрыто тестами, проводится ревью кода, и не поймет иронии :) А вот если вы знаете в своей системе с десяток методов длиннее тысячи строк, десяток классов размером больше пяти тысяч строк…
UFO just landed and posted this here
Он только визуально стал линейным, а по сути та же самая спагетина, только еще и размазанная по сотне методов.
А вы пишете программы без условий и циклов? Тут еще можно выделить методы на каждый блок, только этого не сделать без контекста.
Это не кардинальное решение данного гумно-кода, но чуток улучшает читабельность, т.к. уменьшает количество индентов.
Шило на мыло.
Не скажите.
Структура времени выполнения остается той же — да, верно.
Структура же кода при таком подходе обретает семантику.
Например, при выделении методов им придется давать осмысленные имена.
И не только методам, а еще и параметрам.
А если параметров слишком много — это еще один стимул задуматься, а правильно ли так делать.
Так что от такого шага очень много плюсов.
При этом код времени выполнения будет, скорее всего, таким же.
Структура времени выполнения остается той же — да, верно.
Структура же кода при таком подходе обретает семантику.
Например, при выделении методов им придется давать осмысленные имена.
И не только методам, а еще и параметрам.
А если параметров слишком много — это еще один стимул задуматься, а правильно ли так делать.
Так что от такого шага очень много плюсов.
При этом код времени выполнения будет, скорее всего, таким же.
Насчет return'ов, не уверен, что будет легче сопровождать. Их часто забывают добавлять, а при чтении последующего кода забывают, что где-то на 500 строк выше уже вышли.
1. Если вы пишете код в определенном месте метода, значит он только в этом месте и должен сработать. Если «где-то на 500 строк выше уже вышли», то, скорее всего, вы должны писать код на 500 строк выше.
2. Не забывайте выделять методы, потому что даже 500 строк для метода уж слишком много, я бы ограничился 30-ю строками.
2. Не забывайте выделять методы, потому что даже 500 строк для метода уж слишком много, я бы ограничился 30-ю строками.
У товарища в компании есть интересное правило — метод по количеству строк не должен превышать количество строк ± умещающихся на экран — если больше — внимательно думаем и разбиваем — иначе нарушается визуально-субьективная читаемость кода.
Помнится, он рассказывал, что у них до введения того правила были около полторы сотни методов, в которых была простыня кода.
После рефакторинга, включающего тесты, CI, документирование, переименование большей части методов и переменных и еще кое-чего, оказалось, что половина команды в данном проекте нагружена меньше, чем на половину и они были привлечены на другой проект.
Помнится, он рассказывал, что у них до введения того правила были около полторы сотни методов, в которых была простыня кода.
После рефакторинга, включающего тесты, CI, документирование, переименование большей части методов и переменных и еще кое-чего, оказалось, что половина команды в данном проекте нагружена меньше, чем на половину и они были привлечены на другой проект.
Интересно. А какая у него в компании в среднем диагональ/разрешения монитора?
Насколько я знаю, правило про функцию, умещающуюся в экран, очень старое — даже старше, чем DOS, где на экран помещалось 25 строк максимум по 80 символов в строке. Можно представить, насколько тяжело было программировать в те времена, если даже сейчас, с разрешениями вроде 1920х1080, существуют люди, испытывающие сложности с тем, чтобы вписаться в один экран )
Это, правило, наверное больше относится к куче вложенных блоков, когда код текста из-за блочных отступов может начинаться визуально на правой половине строки, недавно видел вживую такой код…
Тут речь больше о вертикальном размере и лаконичности выражения мысли. То есть, если функция разрослась на несколько экранов, стоит задуматься, действительно ли она выполняет только одно действие или можно выделить некоторые ее блоки в самостоятельные функции. О длине строки тоже, конечно, всегда говорили.
В Пайтоне, кстати говоря, даже есть стандарт на оформление кода, PEP8, где явно указана максимальная длина строки, которая может считаться нормальной.
В Пайтоне, кстати говоря, даже есть стандарт на оформление кода, PEP8, где явно указана максимальная длина строки, которая может считаться нормальной.
Я сейчас делаю плагин к IDEA для автоматического определения мест, где необходимо делать выделение метода, основываясь только на структуре кода.
Результаты очень интересные :-)
Где-то через месяц ожидаю завершение прототипа, мой прогноз — 80% случаев вердикт по выносу в отдельный метод будет верным.
Результаты очень интересные :-)
Где-то через месяц ожидаю завершение прототипа, мой прогноз — 80% случаев вердикт по выносу в отдельный метод будет верным.
+100500
Отлаживать методы с понатыкаными ретурнами геморно.
Я один раз чужой метод смотрел — минут 20 пытался понять, почему отладчик игнорит брейкпойнт. Пока не увидел маленький хитрый ретурн выше :) Хотелось взвыть
Отлаживать методы с понатыкаными ретурнами геморно.
Я один раз чужой метод смотрел — минут 20 пытался понять, почему отладчик игнорит брейкпойнт. Пока не увидел маленький хитрый ретурн выше :) Хотелось взвыть
А встать в начало и пройти по шагам — не?
Значит вы поставили брекпоинт не в том месте. А если минут 20 пытались понять почему код в этом месте не выполняется, то тут не в return дело, а в длине метода, потому что метод в 20-30 строк с return-ами за 20 минут можно обозреть.
Код с ретурнами должен соответствовать одному из следующих случаев:
1) Иметь строгую линейную структуру:
2) Если метод состоит из ровно одного цикла — обеспечивать выход из цикла по заданному условию, стоящему либо в начале, либо в конце цикла (не в середине!)
3) return во всех ветках оператора switch-case (включая default, даже если он не написан явно!).
1) Иметь строгую линейную структуру:
if (...) {
...
return;
}
if (...) {
...
return;
}
if (...) {
...
return;
}
...
2) Если метод состоит из ровно одного цикла — обеспечивать выход из цикла по заданному условию, стоящему либо в начале, либо в конце цикла (не в середине!)
3) return во всех ветках оператора switch-case (включая default, даже если он не написан явно!).
А операторы break и continue в этом подходе имеют право на существование?
На такое я согласен. Но не когда один хитрый ретурн теряется в третьем уровне вложенности
А я не согласен. После запрета goto вариант return из глубокого цикла остался последней возможностью хоть как-то реализовать кванторы («если существуют a,b,c, такие что… то сделать… иначе ....») без введения лишних переменных и ненужного усложнения кода. По крайней мере, пока в языках не реализуют break и continue с глубоких уровней.
Про нэйминг конвеншн написать бы еще… Вымораживает, когда лезешь в код C# и видишь в каждой строчке венгерскую нотацию.
И ведь решарпер не поможет переименовать все безопасно, если говорить о ASP.NET. Во многих местах на айдишники строковые константы завязаны (например, в тестах или JS скриптах).
И ведь решарпер не поможет переименовать все безопасно, если говорить о ASP.NET. Во многих местах на айдишники строковые константы завязаны (например, в тестах или JS скриптах).
С этим сложнее, особенно в API. Но тут надо придерживаться эволюционного подхода. Помечать методы как планируемые к удалению, а после выпуска релиза вычищать все безжалостно. Мне в этом отношении нравится Qt, а там не Java или .Net, а С++ со своей спецификой. И то умудряются поддерживать совместимое API в рамках версии.
Связка строки с именем — скользкая дорожка динамического программирования.
Может в разы уменьшить количество сопровождающего кода, а также автоматически поддерживать изменения, но увеличивает сложность отладки в N раз.
Предпочитаю рефакторить до цепочки if-elseif, вынесенной в отдельный метод, осуществляющей преобразование в нужный мне объект, и только после этого заменять статическую связку if-elseif на динамический reflection.
Получаем относительную безопасность и локализацию подхода с одной стороны, и плюшки минимизации поддержки кода — с другой.
Может в разы уменьшить количество сопровождающего кода, а также автоматически поддерживать изменения, но увеличивает сложность отладки в N раз.
Предпочитаю рефакторить до цепочки if-elseif, вынесенной в отдельный метод, осуществляющей преобразование в нужный мне объект, и только после этого заменять статическую связку if-elseif на динамический reflection.
Получаем относительную безопасность и локализацию подхода с одной стороны, и плюшки минимизации поддержки кода — с другой.
Это все прекрасно описано еще у МакКоннела, Фаулера можно даже (пока) не трогать.
Такой метод рефакторинга напоминает уборку комнаты методом заталкивания всего под кровать.
В том же упомянутом выше Qt такой подход не практикуется.
Основное возражение состоит в том, что разбиение на методы/функции должно производиться не на основании того, какой участок синтаксически выделен в «портянке», а на основание концептаульной обособленности участка кода.
Самый важный фактор читаемости кода — говорящие идентификаторы, по сути, говорящее название часто позволяет вообще не смотреть в реализацию метода/функции.
Но говорящий идентификатор можно назначить только для концептуально обособленного блока, и только на основании такой обособленности можно строить план рефакторинга.
Конечный код должен быть простым, на верхних уровнях он должен складываться в текстовое описание принципов функционирования системы.
По своему опыту скажу, что уже через года 3 практики программирования я почти исключил методы более 1 экрана при написании кода, даже с учетом того, что в моем стиле каждой фигурной скобке отводится своя строка и я отделяю пропуском строки логически обособленные блоки.
Говоря «почти» я имею в виду те функции, где разбиение на функции приводит к усложнению кода, такое бывает, например, в реализации сложных математических методов, в таком случае понятнее оставить функцию, которая реализует формулу/ступень метода целиком, это упростит понимание подготовленному читателю, неподготовленному все равно ничего понять. В таком случае спасают дельные комментарии и отсылки к описанию методов.
Для меня рефакторинг — это переработка некой подсистемы в связи с изменением концепции функционирования системы, то есть переписывание одного набора методов, размером не более половины экрана в другой набор методов в полэкрана, причем методы просты и понятны, все дело в том, что система была построена исходя из ошибочных предположений (это нормально).
В том же упомянутом выше Qt такой подход не практикуется.
Основное возражение состоит в том, что разбиение на методы/функции должно производиться не на основании того, какой участок синтаксически выделен в «портянке», а на основание концептаульной обособленности участка кода.
Самый важный фактор читаемости кода — говорящие идентификаторы, по сути, говорящее название часто позволяет вообще не смотреть в реализацию метода/функции.
Но говорящий идентификатор можно назначить только для концептуально обособленного блока, и только на основании такой обособленности можно строить план рефакторинга.
Конечный код должен быть простым, на верхних уровнях он должен складываться в текстовое описание принципов функционирования системы.
По своему опыту скажу, что уже через года 3 практики программирования я почти исключил методы более 1 экрана при написании кода, даже с учетом того, что в моем стиле каждой фигурной скобке отводится своя строка и я отделяю пропуском строки логически обособленные блоки.
Говоря «почти» я имею в виду те функции, где разбиение на функции приводит к усложнению кода, такое бывает, например, в реализации сложных математических методов, в таком случае понятнее оставить функцию, которая реализует формулу/ступень метода целиком, это упростит понимание подготовленному читателю, неподготовленному все равно ничего понять. В таком случае спасают дельные комментарии и отсылки к описанию методов.
Для меня рефакторинг — это переработка некой подсистемы в связи с изменением концепции функционирования системы, то есть переписывание одного набора методов, размером не более половины экрана в другой набор методов в полэкрана, причем методы просты и понятны, все дело в том, что система была построена исходя из ошибочных предположений (это нормально).
Полностью согласен. Я не призываю бездумно выделять методы. Приведенные два рефакторинга уменьшают уровень вложенности кода, а это лишь первый шаг к улучшению его читабельности.
В свою очередь, стараюсь придерживаться правила «Если ты хочешь написать комметрарий к блоку в функции, то он заслуживает отдельной функции». Функция должна делать одно действие и делать его хорошо. Если в методе несколько сотен, а то и тысяч строк, то тут уж большая его часть делает что-то свое.
Кстати, кажется у Гудлифа, было обсуждение предельного случая разбиения на методы. Если в методе содержится ветвление с двумя ветками, значит метод делает два разных действия. Его надо разделитью Случай еще предельнее — отлов исключений и их обработка в отдельном методе. Но это уже совсем непрактичная крайность.
В свою очередь, стараюсь придерживаться правила «Если ты хочешь написать комметрарий к блоку в функции, то он заслуживает отдельной функции». Функция должна делать одно действие и делать его хорошо. Если в методе несколько сотен, а то и тысяч строк, то тут уж большая его часть делает что-то свое.
Кстати, кажется у Гудлифа, было обсуждение предельного случая разбиения на методы. Если в методе содержится ветвление с двумя ветками, значит метод делает два разных действия. Его надо разделитью Случай еще предельнее — отлов исключений и их обработка в отдельном методе. Но это уже совсем непрактичная крайность.
Такие ветвящиеся деревья if'ов, как в примере, иногда удобно свернуть в СКНФ или СДНФ.
Уменьшается объём, повышается читаемость и логика становится нагляднее.
У меня был случай, когда такого вида метод на 500+ строк свернулся в четырнадцать строчек c условиями СДНФ и вызовом методов. При этом выяснилось, что часть логики была вообще недостижима.
Уменьшается объём, повышается читаемость и логика становится нагляднее.
У меня был случай, когда такого вида метод на 500+ строк свернулся в четырнадцать строчек c условиями СДНФ и вызовом методов. При этом выяснилось, что часть логики была вообще недостижима.
А как выделять новые методы, если среди 500+ строчек циклов и ветвлений прячется еще и несколько десятков локальных переменных? Выносить их в переменные класса? Но они не имеют смысла за пределами выполнения метода, так что незачем им занимать память, да ещё и освобождать их вручную придётся. Выделять метод в отдельный класс? Но тогда будет трудно добраться до переменных того экземпляра, в котором метод содержался изначально. Передавать ссылки на эти переменные из старого метода в новые? Наверное, метод с парой десятков параметров будет тоже сложен для понимания. Собрать переменные в отдельную структуру и передавать ссылку на неё в каждый из методов?
Что на этот счёт говорит теория и практика?
Что на этот счёт говорит теория и практика?
На все ваши вопросы уже есть ответы в рефакторингах Фаулера. Полезное чтиво. Соглашусь, что это не просто, но эффект того стоит.
На этот счет есть рефакторинг Method Object, когда метод выносится в отдельный объект, его локальные переменные становятся полями этого объекта, а тело после этого можно декомпоновать произвольным образом.
Доступ к внутренним поля объекта источника делается через захват
Доступ к внутренним поля объекта источника делается через захват
this, а проблемы видимости решаются средствами языка (например, в C# — через внутренний класс).Не всегда можно средствами языка — это раз.
Не всегда это в принципе нужно — это два.
И вообще, прежде чем подумать — подумай :-)
Не всегда это в принципе нужно — это два.
И вообще, прежде чем подумать — подумай :-)
Вам не надоело писать тривиальные (и потому бессмысленные) советы?
Не надоело. Предпочитаю полную ясность.
Если Вы говорите про частный случай — лучше укажите, что бывают исключения, и в каких случаях.
Описал альтернативный вариант в комментарии ниже.
В языке может не оказаться таких средств. В этом случае используем ограниченный интерфейс, передаваемый в конструкторе — не обязательно весь this, часто им оказывается более узкий интерфейс или вообще адаптер.
Хорошие примеры подобных рефакторингов можно найти в книге «Чистый код» (Clean code).
Вы не согласны, что развернутое описание более полезно?
Если Вы говорите про частный случай — лучше укажите, что бывают исключения, и в каких случаях.
Доступ к внутренним поля объекта источника делается через захват this
Описал альтернативный вариант в комментарии ниже.
проблемы видимости решаются средствами языка
В языке может не оказаться таких средств. В этом случае используем ограниченный интерфейс, передаваемый в конструкторе — не обязательно весь this, часто им оказывается более узкий интерфейс или вообще адаптер.
Хорошие примеры подобных рефакторингов можно найти в книге «Чистый код» (Clean code).
Вы не согласны, что развернутое описание более полезно?
Описал альтернативный вариант в комментарии ниже.
Это «необходимо рассмотреть этот частный случай детально»?
В этом случае используем ограниченный интерфейс, передаваемый в конструкторе — не обязательно весь this, часто им оказывается более узкий интерфейс или вообще адаптер.
Вы считаете, что ограниченный, но публично доступный (потому что в языке может не оказаться и таких средств тоже) интерфейс — лучше?
Вы не согласны, что развернутое описание более полезно?
Развернутое описание надо читать в книжке-первоисточнике.
Да, выносить в отдельный класс. Лучше всего — с максимально ограниченной областью видимости.
Если нужно добираться до переменных экземпляра, внутри которого все это было — передайте их в конструкторе.
Если же их нужно менять… Можно передать в конструкторе простой объект с set-методами. Но если их слишком много… необходимо рассмотреть этот частный случай детально, я не готов решать эту проблему в абстрактном случае.
Если нужно добираться до переменных экземпляра, внутри которого все это было — передайте их в конструкторе.
Если же их нужно менять… Можно передать в конструкторе простой объект с set-методами. Но если их слишком много… необходимо рассмотреть этот частный случай детально, я не готов решать эту проблему в абстрактном случае.
Sign up to leave a comment.
Микро-рефакторинг, о котором мы так часто забываем