Comments 52
Считаю что ключевое — «напиши юнит-тест, потом ломай». Без юниттестов рефакторинг — явная лотерея.
Я вот столкнулся с проблемой что невозможно писать тест т.к. нужен начальный рефакторинг, из-за сильных связанностей, бесконечно запутанных методов… и пр.
выход нашел пока только один — покрыть код регрессионными и приемочными тестами, немного сложнее и тяжеловеснее юнит теста, но хоть какая-то страховочная сеть…
выход нашел пока только один — покрыть код регрессионными и приемочными тестами, немного сложнее и тяжеловеснее юнит теста, но хоть какая-то страховочная сеть…
Вы не поверите, сколько людей делают его на «я запомню» или «у нас на это тестеры есть» :)
применяю такие ноу-хау:
— любой конечный код, который использует глобальные переменные заворачивается в функции (методы) с понятными названиями в которые эти глобальные переменные уже передаются как входящие параметры. Смысл в этом такой: шаг за шагом глобальные переменные «вытесняются» на уровни все выше и выше, и в конце концов «оседают» на своих местах.
— для особо извращенных и запутанных функций создается аналогичная, кошерная. Причем НЕ все места сразу переводятся на новое, а только те, которые точно известно как протестировать (т.е. временно код продублирован). В старой же функции тригеррится варнинг или нотис, так что любое использование «депрекейтнутой» будет замечено и переведено, по мере сил, или по методу 80/20.
— воспринимать старый код как карточный домик, который рухнет если взяться переписывать большой кусок. Локализировать проблему (путем заворачивания в функции без побочных эффектов), и стараться не рушить всё остальное, даже если велик соблазн (разве только если уже виден горизонт). В старом коде не чураться временных свай, затычек, дёрти хаков и дублирования кода. Новый код может временно дублировать старый (пока не все места перешли на новый), внутри же нового кода (который в конце концов захватит все места) — дублирование запретить изначально.
— любой конечный код, который использует глобальные переменные заворачивается в функции (методы) с понятными названиями в которые эти глобальные переменные уже передаются как входящие параметры. Смысл в этом такой: шаг за шагом глобальные переменные «вытесняются» на уровни все выше и выше, и в конце концов «оседают» на своих местах.
— для особо извращенных и запутанных функций создается аналогичная, кошерная. Причем НЕ все места сразу переводятся на новое, а только те, которые точно известно как протестировать (т.е. временно код продублирован). В старой же функции тригеррится варнинг или нотис, так что любое использование «депрекейтнутой» будет замечено и переведено, по мере сил, или по методу 80/20.
— воспринимать старый код как карточный домик, который рухнет если взяться переписывать большой кусок. Локализировать проблему (путем заворачивания в функции без побочных эффектов), и стараться не рушить всё остальное, даже если велик соблазн (разве только если уже виден горизонт). В старом коде не чураться временных свай, затычек, дёрти хаков и дублирования кода. Новый код может временно дублировать старый (пока не все места перешли на новый), внутри же нового кода (который в конце концов захватит все места) — дублирование запретить изначально.
Вы забыли самую главную вешь, без нее все будет прекрасно работать, без нее можно все сделать на ура, но с ней будет быстрей и понятней: %lang% doc… без документации очень плохо :(
вот я щас веду «быдлокод» пхп 4, свн только для бекапа файлы по 5000 строк, куча отчетов в одном файле… рефакторинг плачет по проекту, даже молится о чуде, но вот моих только желаний не достаточно :( и таких проектов много, начали неправильно, поняли свою ошибку а вот добро на рефакторинг заказчик не дает, все ведь и так работает
вот я щас веду «быдлокод» пхп 4, свн только для бекапа файлы по 5000 строк, куча отчетов в одном файле… рефакторинг плачет по проекту, даже молится о чуде, но вот моих только желаний не достаточно :( и таких проектов много, начали неправильно, поняли свою ошибку а вот добро на рефакторинг заказчик не дает, все ведь и так работает
Попробуйте найти подход к заказчику. Ему редко интересно, что разработчику сложно ориентироваться в нагромождении быдлокода, попробуйте объяснить что после рефакторинга время на внесение изменений и дополнительного функционала сократится в несколько раз (соответственно упадёт стоимость этих измений)
К сожалению, бывают такие заказчики, которых под дулом пистолета не переубедить, что добавление юнит-тестов может сохранить многие клетки мозга в будущем, что рефакторинг нужен как воздух. У них одна отмашка — зарелизимся, добавим новых фич, и вот там можно подумать о твоем улучшении кода. Но реально до этого редко доходит. Тыкать в книжку Физерса «Эффективная работа с унаследованным кодом» — себе дороже. Мне пришлось выслушать, что автор этой книги теоретик, и ему далеко до наших реальных проектов, где каждая новая фича — это залог успеха.
В итоге, проект лучше переписать, чем пытаться «осушить болото». Однако такой шаг пагубен для всего проекта. Об этом пишет Физерс, об этом предупреждает Спольски. Замкнутый круг :)
В итоге, проект лучше переписать, чем пытаться «осушить болото». Однако такой шаг пагубен для всего проекта. Об этом пишет Физерс, об этом предупреждает Спольски. Замкнутый круг :)
Ну вот вы и работаете больше, соответственно вам больше платят.
У нас с вами один и тот же заказчик ??? :)
не надо выделять написание unit-тестов в отдельную задачу — это должно быть частью работы над очередной «фичей»
проект ведется с 2005 года, над ним поработало не одно поколение программеров, как умные так и балбесы, рефакторинг в данном случае — огромный простой с точки зрения заказчика
Аналогично есть проект которому больше 7 лет, написано с нагромождением iframe'ов друг в друга. В те далекие времена это считалось ajaxом =( И переписать не дают и количество неявных связей и зависимостей между частями столько за это время создалось, что рефакторинг только микрошажками возможен и смысла особого нет. Остаётся только вооружившись мачете каждый раз пробираться сквозь быдлокод, исправляя очередной баг или расширяя функционал.
рефакторинг только микрошажками возможен и смысла особого нет
А вот в том, что смысла нет — вы не правы. Да, рефакторинг микрошажками может занять больше времени, чем переписывание с нуля (хотя, кстати, не факт), но у него есть огромнейшее преимущество с точки зрения бизнеса — добавление новых фич не останавливается, т.е. нет выпадания из бизнеса. А если каждое изменение хоть немного, но улучшает ситуацию (или хотя бы не ухудшает), то рано или поздно проект станет идеальным :-)
Хорошие тесты и есть документация!
Отличные правила.
В качестве дополнения могу люто, бешено порекомендовать www.google.ru/search?q=ISBN+5-93286-045-6 «Рефакторинг: улучшение существующего кода» Фаулера. Это отличная кандидатура на звание настольной книги :)
В качестве дополнения могу люто, бешено порекомендовать www.google.ru/search?q=ISBN+5-93286-045-6 «Рефакторинг: улучшение существующего кода» Фаулера. Это отличная кандидатура на звание настольной книги :)
Ну, я скажу только за себя. Рефакторинг зависит очень от самого проекта. Но я чаще всего начинал с крошек облегчающих жизнь. Крошки собирал в мякиши, а из мякишей делал чернильниу, чтобы как ленин в тюрме, писать новое молоком )
Вот блин загнул, все — пора спать.
Вот блин загнул, все — пора спать.
А мой маленький секрет — комментировать, что функция делает. Например.
// Отсоединяет цепочку объектов. Цепочка остаётся цепочкой,
// но её концы начинают указывать в nil, а получившаяся
// «дыра» закрывается.
// Вход:
// aChainStart — начало цепочки
// aChainEnd — начало цепочки
// Сложность: константная
// Потокоизоляция: условно-безопасно
// Предположения:
// * aChainStart и aChainEnd принадлежат списку (либо оба nil,
// тогда ничего не делаем) и находятся в правильном порядке.
procedure UnlinkChain(
const aChainStart, aChainEnd: TDequeItem);
(из модуля работы со связными списками)
// Отсоединяет цепочку объектов. Цепочка остаётся цепочкой,
// но её концы начинают указывать в nil, а получившаяся
// «дыра» закрывается.
// Вход:
// aChainStart — начало цепочки
// aChainEnd — начало цепочки
// Сложность: константная
// Потокоизоляция: условно-безопасно
// Предположения:
// * aChainStart и aChainEnd принадлежат списку (либо оба nil,
// тогда ничего не делаем) и находятся в правильном порядке.
procedure UnlinkChain(
const aChainStart, aChainEnd: TDequeItem);
(из модуля работы со связными списками)
P.S. Если не получается столь сжато написать — возможно, придётся или переписать функцию, или наладить дополнительную проверку корректности…
написание документации — самое худшее что можно заставить делать программиста (с)кто-то с хабра
вообще хороший подход :) а еще один умный человек один раз мне сказал: если в функции больше 30-50 строк, значит она не правильная…
вообще хороший подход :) а еще один умный человек один раз мне сказал: если в функции больше 30-50 строк, значит она не правильная…
Если бы вы еще юзали синтаксис Doxygen, цены бы вам не было.
UFO just landed and posted this here
комментарии — ладно
у нас вот имена полей в таблицах и классах, и сами классы и таблицы не на английском для совместимости с ERP (чтобы потом не приходилось мучительно вспоминать английский перевод), а как забавно видеть и использовать такие методы, особенно вперемешку с английскими :)
у нас вот имена полей в таблицах и классах, и сами классы и таблицы не на английском для совместимости с ERP (чтобы потом не приходилось мучительно вспоминать английский перевод), а как забавно видеть и использовать такие методы, особенно вперемешку с английскими :)
если разработчик плохо пишет по английски, то лучшее уж комментарии будут по-русски, понятнее будет о чем речь :-)
«Если разработчик (к примеру, немец) плохо пишет по-английски, то лучше уж комментарии будут по-немецки, понятнее будет, о чём речь». Всё хорошо, пока этот код не достался, к примеру, Вам — а Вы из всего немецкого знаете только «йа-йа», «дас ист фантастиш» и «ин гретен фамилен клювен нихт клацен».
Лично я предпочту ломаный английский превосходному немецкому/китайскому/хинди/etc.
Лично я предпочту ломаный английский превосходному немецкому/китайскому/хинди/etc.
+1. Вот пример из проекта Krysseliste — норвежский, кажется.
Private Sub lagrevaregruppe()
'kan bare v?re 7 aktive varegrupper
If Not varegruppe.aktiv = cbVaregruppeAktiv.Checked And varegruppe.aktiv = False Then
If varegrupper.Where(Function(n) n.aktiv And n.ajminonly = False).Count >= 7 Then
MsgBox("Det kan bare v?re sju aktive varegrupper")
Exit Sub
End If
End If
With varegruppe
.varegruppenavn = txtVaregruppenavn.Text
.varegruppebilde = txtVaregruppeBilde.Text
.varegruppebeskrivelse = txtVaregruppeBeskrivelse.Text
.adminonly = cbVaregruppeAdminOnly.Checked
.aktiv = cbVaregruppeAktiv.Checked
.sted = driftsted
End With
* This source code was highlighted with Source Code Highlighter.
Я бы еще добавил обязательно: описание входных параметров, возвращаемого значения и побочных эффектов.
Спасибо! Огромное спасибо. Обязательно покажу статью своим командам. Может быть, наконец, возьмутся дорефакторить оставшиеся древние модули, которые всем уже плешь проели своими глюками.
P.S. Очень рад, что пригласил Вас на Хабр :)
P.S. Очень рад, что пригласил Вас на Хабр :)
Только купил Физерса, ещё не читал, но если ваш пост это выжимка из книги, то к большинству рецептов пришол сам. 1,5 годичный проэкт за три месяца отрефакторил вдоль и впоперёк.
Но в вашей статье нету главного. Любой рефакторинг должен иметь цель, не страшно если во время рефакторинга цель поменялась, но целью не должен быть сам рефакториг(должна быть добавленая стоимость для проэкта), конечно промежуточные шаги могут просто почистить код от закоментированых сточек, но хорошая конечная цель, например, обособить модуль А, сделать его независимой библиотекой со своими тестами.
Слишком часто новички увлекаются рефакторингом ради рефакторинга занимаются неприрывным «улушением» кода вместо реализации приоритетных задач, наличие автоматическтого рафакторинга в современных IDE очень способствует такому положению дел.
Но в вашей статье нету главного. Любой рефакторинг должен иметь цель, не страшно если во время рефакторинга цель поменялась, но целью не должен быть сам рефакториг(должна быть добавленая стоимость для проэкта), конечно промежуточные шаги могут просто почистить код от закоментированых сточек, но хорошая конечная цель, например, обособить модуль А, сделать его независимой библиотекой со своими тестами.
Слишком часто новички увлекаются рефакторингом ради рефакторинга занимаются неприрывным «улушением» кода вместо реализации приоритетных задач, наличие автоматическтого рафакторинга в современных IDE очень способствует такому положению дел.
После огромного количества сайтов, сделанных различных поставщикам техники в нашей области, встретить на главной фото Т-170Б производства нашего ЧТЗ. Это не для слабонервных, скажу я вам :)
И все восхищаются. Предлагаю тем, кто открыл для себя новый мир — прочитать книгу Martin Fowler «Refactoring: Improving the Design of Existing Code», как уже советовали выше. Жить станет проще.
А то, кто ее читал, может почитать про более узкие типы рефакторингов Fowler books
А то, кто ее читал, может почитать про более узкие типы рефакторингов Fowler books
[смахивает слезу рукавом тельняшки]
Тёзка! Я тя практически абажаю! Подписываюсь под каждым словом!
Так уж выходило, что почти на половине проектов приходилось иметь дело с тоннами чужого кода. В одном случае код вообще писался даже на протяжении 4 или 5 лет разными программистами. Просто каша — ни в борщ, ни в Красную Армию. Это было занятие не для слабонервных. Причем, всё работало и очень даже резво. Но при попытке что-то внести — разумеется, тянуло всё за собой. В общем, применяя практики, описанные в статье (за исключением юнит-тестов, увы), постепенно переписал половину системы и сделал возможность расширять её и дальше — без головняка, геморроя и прочей медицины.
От себя добавлю, что рефакторинг небольшого куска функционала, с одновременным добавлением чего-то нового — лучший способ разобраться в наследованном коде.
Приходишь на проект, осматриваешься, понимаешь общий принцип построения архитектуры. А далее, берешь самую небольшую задачу и не торопясь разбираешь систему на винтики, глядя — где же можно чего подкрутить, чтобы задачу решить. Ну и потом собрать на место всё так, чтобы завелось с первого раза :)
Две-три такие задачи — и начинаешь понимать, в каком направлении вести рефакторинг, где самые слабые места, на которые надо обратить внимание в первую очередь, где ещё пока терпимо и можно не трогать до поры до времени… А куда вообще лучше не соваться, понимая, что переписать там что-то невозможно без переделки половины системы. В этих случаях надо просто смириться и тратить силы на более важные направления.
В общем, зачет, пиши ещё.
P.S. к вопросу о source control и бранчах — вот очень неплохая обзорная статья по базовым практикам
www.cmcrossroads.com/bradapp/acme/branching/
Практики эти применимы независимо от выбранной системы. Будут вопросы — обращайтесь.
Тёзка! Я тя практически абажаю! Подписываюсь под каждым словом!
Так уж выходило, что почти на половине проектов приходилось иметь дело с тоннами чужого кода. В одном случае код вообще писался даже на протяжении 4 или 5 лет разными программистами. Просто каша — ни в борщ, ни в Красную Армию. Это было занятие не для слабонервных. Причем, всё работало и очень даже резво. Но при попытке что-то внести — разумеется, тянуло всё за собой. В общем, применяя практики, описанные в статье (за исключением юнит-тестов, увы), постепенно переписал половину системы и сделал возможность расширять её и дальше — без головняка, геморроя и прочей медицины.
От себя добавлю, что рефакторинг небольшого куска функционала, с одновременным добавлением чего-то нового — лучший способ разобраться в наследованном коде.
Приходишь на проект, осматриваешься, понимаешь общий принцип построения архитектуры. А далее, берешь самую небольшую задачу и не торопясь разбираешь систему на винтики, глядя — где же можно чего подкрутить, чтобы задачу решить. Ну и потом собрать на место всё так, чтобы завелось с первого раза :)
Две-три такие задачи — и начинаешь понимать, в каком направлении вести рефакторинг, где самые слабые места, на которые надо обратить внимание в первую очередь, где ещё пока терпимо и можно не трогать до поры до времени… А куда вообще лучше не соваться, понимая, что переписать там что-то невозможно без переделки половины системы. В этих случаях надо просто смириться и тратить силы на более важные направления.
В общем, зачет, пиши ещё.
P.S. к вопросу о source control и бранчах — вот очень неплохая обзорная статья по базовым практикам
www.cmcrossroads.com/bradapp/acme/branching/
Практики эти применимы независимо от выбранной системы. Будут вопросы — обращайтесь.
Вот тут все Фаулера рекомендуют, а я порекомендую Макконнела :)
Больше всего мне понравилась методика «чистый идеальный мир» — интерфейс — «ад», где он рекомендует определить интерфейс между грязным кодом и чистым идеальным кодом, и потихоньку переносить грязный код за «барьер».
Ну и вообще, цитата из книжки:
Значение слова «рефакторинг» довольно размыто: так называют исправление дефектов, реализацию новой функциональности, модификацию проекта — по сути любое изменение кода. Это неуместно. Целенаправленный процесс изменений может быть эффективной стратегией, обеспечивающей постепенное повышение качества программы при ее сопровождении и предотвращающей всем известную смертельную спираль энтропии ПО, но само по себе изменение достоинств не имеет.
Больше всего мне понравилась методика «чистый идеальный мир» — интерфейс — «ад», где он рекомендует определить интерфейс между грязным кодом и чистым идеальным кодом, и потихоньку переносить грязный код за «барьер».
Ну и вообще, цитата из книжки:
Значение слова «рефакторинг» довольно размыто: так называют исправление дефектов, реализацию новой функциональности, модификацию проекта — по сути любое изменение кода. Это неуместно. Целенаправленный процесс изменений может быть эффективной стратегией, обеспечивающей постепенное повышение качества программы при ее сопровождении и предотвращающей всем известную смертельную спираль энтропии ПО, но само по себе изменение достоинств не имеет.
я начинаю рефакторинг кода с групировки кусков кода, чтоб все что должно быть вместе/рядом — было рядом. в данном примере ооп рулит.
Если кого-то заинтересуют нетривиальные вопросы рефакторинга, выходящие за рамки букваря, то рекомендую доклад коллеги
Безудержный Рефакторинг: как не убить себя об стену.
Кстати, Физерс тоже там выступал с «мастер-классом» и выглядел, скажем, так себе — все у него падало и глючило.
Видео у меня есть, но он просил вроде как не публиковать этот фэйл.
В общем, он тренер-теоретик.
Безудержный Рефакторинг: как не убить себя об стену.
Кстати, Физерс тоже там выступал с «мастер-классом» и выглядел, скажем, так себе — все у него падало и глючило.
Видео у меня есть, но он просил вроде как не публиковать этот фэйл.
В общем, он тренер-теоретик.
Sign up to leave a comment.
Рефакторинг: миссия (не)выполнима?