Рефакторинг: миссия (не)выполнима?

    Что делать, если исходники проекта напоминают болото, а жить в нем планируется еще долго? Рефакторинг исходных кодов проекта — это более выгодная из двух альтернатив разобраться с означенной проблемой. Вторая из альтернатив — выбросить и переписать все заново — обычно не устраивает, по разным причинам.

    А как делать этот самый рефакторинг, если даже неизвестно, с чего начать? Как осушить болото кода и не утонуть нем?

    Конечно, идеальный вариант — считать рефакторинг одной из активности на проекте с самого его начала и выделять на него проектное время команды. В жизни, как ни странно, однако чаще всего оказывается, что предыдущий лид проекта не сильно озадачивался этим вопросом, предпочитая плыть по течению; возможно, он надеялся когда-нибудь сбагрить эту ношу менее удачливому лиду — пока проект еще не развалился; а может быть, просто не знал, что делать. Шеф, усё пропало!

    Расскажу, как делаю я.

    Правило номер 1: рефакторинг — это процесс. Поэтому, прежде всего, нужно сдружиться с мыслью, что переделать на «зашибись» не получится ни через неделю, ни через две. После осознания этого факта берем книгу М.Физерса, «Эффективная работа с унаследованным кодом» (Working Effectively With Legacy Code), и начинаем с ней знакомиться. Одновременно удостоверяемся, что хорошо освоились с source control, который у нас используется, и параллельно чтению книги приводим в порядок код (первичная «зачистка»): удаляем закомментированный и «мертвый» код, ему не место в проекте; структурируем «кашу» по файлам; привОдим в соответствие сoding convention. Более серьезных переделок на этом этапе лучше не проводить. Если есть возможность писать юнит-тесты — следует над этим подумать (но не делать, так как обычно на этом этапе писать их пока бесполезно, давайте терпеть до следующего :).

    Правило номер 2: source control всему голова. Вы будете смеяться, если узнаете, как много людей не знаю, что такое бранч. Хорошее знание source control поможет не похерить окончательно проект в случае, если некоторые из ваших многочисленных изменений «несовместимы с жизнью». В такой ситуации только rollback спасет отца русской демократии. Собсна, я рекомендую известный труд SvnBook тыц, особенно по части бранчей, даже если у вас не svn. Также можно поискать в сети слайды Source control & Agile и тыц. Очень весело про source control написано тут.

    Правило номер 3: автоматизируйте рутину, это примета — к счатью :) Это я к тому, что стоит обзавестить какой-нибудь системой автобилда, чтобы собрать проект можно было в 1 клик. Такая система — реально плюс к морали, потому что я не раз встречался с ситуацией, когда проект нужно было строить руками из студии, полчаса копируя артефакты в сложной последовательности по странным расшаренным папкам в сети, и такая ситуация очень сильно отбивала у людей охоту что-то менять. Я долгое время пользовался CruiseControl (рекомендую), счас используется TeamBuild из TFS (политика партии), но непринципиально — для начала сойдет и make/ant/nant, запускаемый из консоли.

    К тому моменту, как книгу дочиталась, codebase должна выглядеть прилично, чтобы, по крайней мере, не тошнило от исходников, а также легко пересобираться. Конечно, до ощущения красоты проекта еще ой как далеко, да и архитектура не то чтобы хромает — еле ходит :)

    Правило номер 4: начинаейте с малого, но хорошего. А именно: дальнейшая стратегия работы с проектом следующая: для той части проекта, над которой мы работаем счас, выделяем «песочницу», в которой все будет «как надо» — изолируем ее от остальной части кода, пишем ее _хорошо_ — ООП, best practices в руки. Весьма желательны юнит-тесты. Весьма — потому что делать рефакторинг на меткий глаз и честное слово лучше вообще никогда :)

    По мере написания своего кода станет ясно, что нашей подсистеме так или иначе придется обращаться к остальному коду некоторым способом. Эту поверхность взаимодействия выделяем в отдельный API (для .net, например, я создаю статический класс, через методы которого мой код взаимодействует со старым) и также пишем unit-testы для него. Как именно его сделаеть и насколько аккуратно — это непринципиально, потому что мы от него постепенно избавимся. После того, как наш «красивый» код успешно работает с этим API, и есть тесты для этого API, можете заглянуть за него, и рефакторить существующее за ним «мясо» согласно книге, которую вы прочли, продолжая покрывать тестами вглубь. В книге, на которую я сослался, также описаны приемы, которые могут помочь, если вы не начинаете с собственной песочницы, а дополняете функционалом существующий (плохо)код.
    В общем, отсюда плавно истекает (болото же ж, как-никак!) правило 5: работает — не трогай, а напиши юнит-тест, потом ломай. И то — если наш код туда как-то лезет.

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

    Чуть не забыл — на каждом этапе вашего долгого (не сомневайтесь;) пути вам могут быть полезны инструменты автоматического анализа кода — (Visual)NDepend, FxCop и Gendarme для .net/mono, simian, всякие line counters и прочие (lint, например, если вы С/С++). Java-, php- и остальные товарищи могут вписать требуемое.

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

    Комментарии 52

      +3
      Считаю что ключевое — «напиши юнит-тест, потом ломай». Без юниттестов рефакторинг — явная лотерея.
        +2
        Я вот столкнулся с проблемой что невозможно писать тест т.к. нужен начальный рефакторинг, из-за сильных связанностей, бесконечно запутанных методов… и пр.
        выход нашел пока только один — покрыть код регрессионными и приемочными тестами, немного сложнее и тяжеловеснее юнит теста, но хоть какая-то страховочная сеть…
          +1
          Вы не поверите, сколько людей делают его на «я запомню» или «у нас на это тестеры есть» :)
          +3
          применяю такие ноу-хау:
          — любой конечный код, который использует глобальные переменные заворачивается в функции (методы) с понятными названиями в которые эти глобальные переменные уже передаются как входящие параметры. Смысл в этом такой: шаг за шагом глобальные переменные «вытесняются» на уровни все выше и выше, и в конце концов «оседают» на своих местах.
          — для особо извращенных и запутанных функций создается аналогичная, кошерная. Причем НЕ все места сразу переводятся на новое, а только те, которые точно известно как протестировать (т.е. временно код продублирован). В старой же функции тригеррится варнинг или нотис, так что любое использование «депрекейтнутой» будет замечено и переведено, по мере сил, или по методу 80/20.
          — воспринимать старый код как карточный домик, который рухнет если взяться переписывать большой кусок. Локализировать проблему (путем заворачивания в функции без побочных эффектов), и стараться не рушить всё остальное, даже если велик соблазн (разве только если уже виден горизонт). В старом коде не чураться временных свай, затычек, дёрти хаков и дублирования кода. Новый код может временно дублировать старый (пока не все места перешли на новый), внутри же нового кода (который в конце концов захватит все места) — дублирование запретить изначально.
          • НЛО прилетело и опубликовало эту надпись здесь
              0
              Согласен. Наверное имеет смысл разбить код по независимым частям и поручить рефакторить каждую только одному программисту. Начать с написания интерфейса-границы между ними.
            0
            Вы забыли самую главную вешь, без нее все будет прекрасно работать, без нее можно все сделать на ура, но с ней будет быстрей и понятней: %lang% doc… без документации очень плохо :(
            вот я щас веду «быдлокод» пхп 4, свн только для бекапа файлы по 5000 строк, куча отчетов в одном файле… рефакторинг плачет по проекту, даже молится о чуде, но вот моих только желаний не достаточно :( и таких проектов много, начали неправильно, поняли свою ошибку а вот добро на рефакторинг заказчик не дает, все ведь и так работает
              +1
              Попробуйте найти подход к заказчику. Ему редко интересно, что разработчику сложно ориентироваться в нагромождении быдлокода, попробуйте объяснить что после рефакторинга время на внесение изменений и дополнительного функционала сократится в несколько раз (соответственно упадёт стоимость этих измений)
                +1
                К сожалению, бывают такие заказчики, которых под дулом пистолета не переубедить, что добавление юнит-тестов может сохранить многие клетки мозга в будущем, что рефакторинг нужен как воздух. У них одна отмашка — зарелизимся, добавим новых фич, и вот там можно подумать о твоем улучшении кода. Но реально до этого редко доходит. Тыкать в книжку Физерса «Эффективная работа с унаследованным кодом» — себе дороже. Мне пришлось выслушать, что автор этой книги теоретик, и ему далеко до наших реальных проектов, где каждая новая фича — это залог успеха.

                В итоге, проект лучше переписать, чем пытаться «осушить болото». Однако такой шаг пагубен для всего проекта. Об этом пишет Физерс, об этом предупреждает Спольски. Замкнутый круг :)
                  0
                  Ну вот вы и работаете больше, соответственно вам больше платят.
                    +3
                    Да уж, кидайте мусор мимо урны — будет уборщице работа
                    +1
                    У нас с вами один и тот же заказчик ??? :)
                      +1
                      не надо выделять написание unit-тестов в отдельную задачу — это должно быть частью работы над очередной «фичей»
                        0
                        несомненно, но в моем случае имелось в виду, что их надо написать для уже существующего монструозного кода, чтобы можно было приступить к безопасному рефакторингу
                          +1
                          надо проводить рефакторинг в рамках задачи, решению которой он помогает. Рефакторинг сам по себе без точно поставленной цели не всегда оправдан.
                      0
                      проект ведется с 2005 года, над ним поработало не одно поколение программеров, как умные так и балбесы, рефакторинг в данном случае — огромный простой с точки зрения заказчика
                        0
                        Аналогично есть проект которому больше 7 лет, написано с нагромождением iframe'ов друг в друга. В те далекие времена это считалось ajaxом =( И переписать не дают и количество неявных связей и зависимостей между частями столько за это время создалось, что рефакторинг только микрошажками возможен и смысла особого нет. Остаётся только вооружившись мачете каждый раз пробираться сквозь быдлокод, исправляя очередной баг или расширяя функционал.
                          0
                          рефакторинг только микрошажками возможен и смысла особого нет

                          А вот в том, что смысла нет — вы не правы. Да, рефакторинг микрошажками может занять больше времени, чем переписывание с нуля (хотя, кстати, не факт), но у него есть огромнейшее преимущество с точки зрения бизнеса — добавление новых фич не останавливается, т.е. нет выпадания из бизнеса. А если каждое изменение хоть немного, но улучшает ситуацию (или хотя бы не ухудшает), то рано или поздно проект станет идеальным :-)
                      0
                      Хорошие тесты и есть документация!
                      +2
                      Отличные правила.

                      В качестве дополнения могу люто, бешено порекомендовать www.google.ru/search?q=ISBN+5-93286-045-6 «Рефакторинг: улучшение существующего кода» Фаулера. Это отличная кандидатура на звание настольной книги :)
                        0
                        Так да )
                          0
                          Таки да
                          +2
                          Ну, я скажу только за себя. Рефакторинг зависит очень от самого проекта. Но я чаще всего начинал с крошек облегчающих жизнь. Крошки собирал в мякиши, а из мякишей делал чернильниу, чтобы как ленин в тюрме, писать новое молоком )

                          Вот блин загнул, все — пора спать.
                            +1
                            А мой маленький секрет — комментировать, что функция делает. Например.

                            // Отсоединяет цепочку объектов. Цепочка остаётся цепочкой,
                            // но её концы начинают указывать в nil, а получившаяся
                            // «дыра» закрывается.
                            // Вход:
                            // aChainStart — начало цепочки
                            // aChainEnd — начало цепочки
                            // Сложность: константная
                            // Потокоизоляция: условно-безопасно
                            // Предположения:
                            // * aChainStart и aChainEnd принадлежат списку (либо оба nil,
                            // тогда ничего не делаем) и находятся в правильном порядке.
                            procedure UnlinkChain(
                            const aChainStart, aChainEnd: TDequeItem);

                            (из модуля работы со связными списками)
                              +1
                              P.S. Если не получается столь сжато написать — возможно, придётся или переписать функцию, или наладить дополнительную проверку корректности…
                                +1
                                написание документации — самое худшее что можно заставить делать программиста (с)кто-то с хабра
                                вообще хороший подход :) а еще один умный человек один раз мне сказал: если в функции больше 30-50 строк, значит она не правильная…
                                  +3
                                  Если бы вы еще юзали синтаксис Doxygen, цены бы вам не было.
                                    0
                                    Delphi поддерживает? В другом проекте, на C++, так оно и есть.
                                • НЛО прилетело и опубликовало эту надпись здесь
                                    0
                                    комментарии — ладно
                                    у нас вот имена полей в таблицах и классах, и сами классы и таблицы не на английском для совместимости с ERP (чтобы потом не приходилось мучительно вспоминать английский перевод), а как забавно видеть и использовать такие методы, особенно вперемешку с английскими :)
                                      0
                                      если разработчик плохо пишет по английски, то лучшее уж комментарии будут по-русски, понятнее будет о чем речь :-)
                                        0
                                        «Если разработчик (к примеру, немец) плохо пишет по-английски, то лучше уж комментарии будут по-немецки, понятнее будет, о чём речь». Всё хорошо, пока этот код не достался, к примеру, Вам — а Вы из всего немецкого знаете только «йа-йа», «дас ист фантастиш» и «ин гретен фамилен клювен нихт клацен».

                                        Лично я предпочту ломаный английский превосходному немецкому/китайскому/хинди/etc.
                                          0
                                          +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.
                                      0
                                      Я бы еще добавил обязательно: описание входных параметров, возвращаемого значения и побочных эффектов.
                                      +1
                                      Спасибо! Огромное спасибо. Обязательно покажу статью своим командам. Может быть, наконец, возьмутся дорефакторить оставшиеся древние модули, которые всем уже плешь проели своими глюками.

                                      P.S. Очень рад, что пригласил Вас на Хабр :)
                                        0
                                        а вы в сикужи работаете?:)
                                          0
                                          Ага
                                            0
                                            жалко вы весь.нет у себя свернули. что-то у меня вся программерская жизнь рядом с вами :)
                                              0
                                              Ну свернули не весь, но много. Сервера реального времени на дотнете как-то не пошли :))
                                              А GUI, вроде как до сих пор пишут на .NET. Правда, проекты не большие.
                                          0
                                          *смущается* :) И вам спасибо
                                            0
                                            Да чего смущаться-то :) Кое-кто из других приглашенных в минус ушел или не пишет ничего, а у Вас пошло :)
                                          +2
                                          Только купил Физерса, ещё не читал, но если ваш пост это выжимка из книги, то к большинству рецептов пришол сам. 1,5 годичный проэкт за три месяца отрефакторил вдоль и впоперёк.
                                          Но в вашей статье нету главного. Любой рефакторинг должен иметь цель, не страшно если во время рефакторинга цель поменялась, но целью не должен быть сам рефакториг(должна быть добавленая стоимость для проэкта), конечно промежуточные шаги могут просто почистить код от закоментированых сточек, но хорошая конечная цель, например, обособить модуль А, сделать его независимой библиотекой со своими тестами.
                                          Слишком часто новички увлекаются рефакторингом ради рефакторинга занимаются неприрывным «улушением» кода вместо реализации приоритетных задач, наличие автоматическтого рафакторинга в современных IDE очень способствует такому положению дел.
                                            0
                                            Не, это не выжимки из книги, так что купили вы ее не зря ) Насчет непрерывного улучшайзинга — это да, рефакторинг ради рефакторинга слишком бестолково и расточительно.
                                            +2
                                            После огромного количества сайтов, сделанных различных поставщикам техники в нашей области, встретить на главной фото Т-170Б производства нашего ЧТЗ. Это не для слабонервных, скажу я вам :)
                                              0
                                              И все восхищаются. Предлагаю тем, кто открыл для себя новый мир — прочитать книгу Martin Fowler «Refactoring: Improving the Design of Existing Code», как уже советовали выше. Жить станет проще.

                                              А то, кто ее читал, может почитать про более узкие типы рефакторингов Fowler books
                                                +2
                                                [смахивает слезу рукавом тельняшки]
                                                Тёзка! Я тя практически абажаю! Подписываюсь под каждым словом!

                                                Так уж выходило, что почти на половине проектов приходилось иметь дело с тоннами чужого кода. В одном случае код вообще писался даже на протяжении 4 или 5 лет разными программистами. Просто каша — ни в борщ, ни в Красную Армию. Это было занятие не для слабонервных. Причем, всё работало и очень даже резво. Но при попытке что-то внести — разумеется, тянуло всё за собой. В общем, применяя практики, описанные в статье (за исключением юнит-тестов, увы), постепенно переписал половину системы и сделал возможность расширять её и дальше — без головняка, геморроя и прочей медицины.

                                                От себя добавлю, что рефакторинг небольшого куска функционала, с одновременным добавлением чего-то нового — лучший способ разобраться в наследованном коде.
                                                Приходишь на проект, осматриваешься, понимаешь общий принцип построения архитектуры. А далее, берешь самую небольшую задачу и не торопясь разбираешь систему на винтики, глядя — где же можно чего подкрутить, чтобы задачу решить. Ну и потом собрать на место всё так, чтобы завелось с первого раза :)

                                                Две-три такие задачи — и начинаешь понимать, в каком направлении вести рефакторинг, где самые слабые места, на которые надо обратить внимание в первую очередь, где ещё пока терпимо и можно не трогать до поры до времени… А куда вообще лучше не соваться, понимая, что переписать там что-то невозможно без переделки половины системы. В этих случаях надо просто смириться и тратить силы на более важные направления.

                                                В общем, зачет, пиши ещё.

                                                P.S. к вопросу о source control и бранчах — вот очень неплохая обзорная статья по базовым практикам
                                                www.cmcrossroads.com/bradapp/acme/branching/
                                                Практики эти применимы независимо от выбранной системы. Будут вопросы — обращайтесь.

                                                  +3
                                                  Вот тут все Фаулера рекомендуют, а я порекомендую Макконнела :)

                                                  Больше всего мне понравилась методика «чистый идеальный мир» — интерфейс — «ад», где он рекомендует определить интерфейс между грязным кодом и чистым идеальным кодом, и потихоньку переносить грязный код за «барьер».

                                                  Ну и вообще, цитата из книжки:
                                                  Значение слова «рефакторинг» довольно размыто: так называют исправление дефектов, реализацию новой функциональности, модификацию проекта — по сути любое изменение кода. Это неуместно. Целенаправленный процесс изменений может быть эффективной стратегией, обеспечивающей постепенное повышение качества программы при ее сопровождении и предотвращающей всем известную смертельную спираль энтропии ПО, но само по себе изменение достоинств не имеет.
                                                    +2
                                                    Да, Макконел — эта зверь :) Я уже и жену свою заставил его читать. Теперь она его практически любит :))
                                                      +1
                                                      Жену-то зачем? :)
                                                        0
                                                        Она тоже разработчик — Action script, Flex, немного C# и Ada :)
                                                  +1
                                                  я начинаю рефакторинг кода с групировки кусков кода, чтоб все что должно быть вместе/рядом — было рядом. в данном примере ооп рулит.
                                                    0
                                                    Если кого-то заинтересуют нетривиальные вопросы рефакторинга, выходящие за рамки букваря, то рекомендую доклад коллеги
                                                    Безудержный Рефакторинг: как не убить себя об стену.

                                                    Кстати, Физерс тоже там выступал с «мастер-классом» и выглядел, скажем, так себе — все у него падало и глючило.
                                                    Видео у меня есть, но он просил вроде как не публиковать этот фэйл.
                                                    В общем, он тренер-теоретик.

                                                    Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                                                    Самое читаемое