Мы попытались провести рефакторинг Hudson.java, но безуспешно; только позже я смог успешно провести рефакторинг, благодаря опыту с первой попытки и большему количеству времени. В любом случае это была отличная возможность для обучения.
Уроки выучены
Две самые важные вещи, которые мы узнали:
- Никогда не стоит недооценивать устаревший код. Он более сложный и переплетенный, чем вы ожидаете, и в его рукавах больше неприятных сюрпризов, чем вы можете себе представить.
- Никогда не стоит недооценивать устаревший код.
И еще один важный момент: когда вы устали и впали в депрессию, получайте удовольствие от чтения « лучших комментариев за всю историю » в StackOverflow :-). Видя чужое страдание, кажется, что его собственное становится меньше.
Я также начал думать, что процесс рефакторинга должен быть более строгим, чтобы защитить вас от чрезмерного отклонения от своей первоначальной цели и от потери в вечном цикле исправления чего-либо <-> обнаружения новых проблем. Люди имеют тенденцию вносить изменения рефакторинга в глубину, которые могут легко сбить их с пути, далеко от того, что им действительно нужно; важно периодически останавливаться и смотреть на то, где мы находимся, где мы пытаемся добраться и не теряемся ли мы, и не нужно просто сокращать текущую «ветвь» рефакторингов и возвращаться к какой-то более ранней точке и попробовать, возможно, совершенно другое решение. Я полагаю, что одним из ключевых преимуществ метода Микадо является то, что он предоставляет вам этот глобальный обзор — который легко теряется, когда он только в вашей голове — и с точками отката.
Зло Кодекса Наследия
Ради бога, используйте систему внедрения зависимостей! Синглтоны и их ручной поиск действительно усложняют тестирование и влияют на гибкость кода.
Не используйте открытые поля. Они действительно затрудняют замену класса интерфейсом.
Отражение и многопоточность делают довольно трудным, если не невозможным, выявление зависимостей конкретного фрагмента кода и, следовательно, последствий его изменения. Я бы с трудом узнал все места, где вызывается Hudson.getInstance, пока его конструктор еще работает.
Наш путь к неудаче и успеху
Существует много способов рефакторинга, которые можно выполнить с помощью Hudson.java, поскольку это типичный класс Бога, который дополнительно распространяет свои щупальца по всей базе кода посредством своего злого одноэлементного экземпляра, который используется кем угодно для самых разных целей. Гойко описывает некоторые проблемы, которые стоит устранить .
Провал
Мы попытались начать с малого и «нормализовать» инициализацию синглтона, что делается не в фабричном методе, а в самом конструкторе. Я не очень хорошо выбрал цель, так как она не приносит особой пользы. Идея состояла в том, чтобы сделать возможным иметь потенциально и другие реализации Hudson — например, MockHudson — но в отношении состояния кода это не было реально осуществимо, и даже если бы это было так, простого Hudson.setInstance, возможно, было бы достаточно. В любом случае мы пытались создать фабричный метод и перенести инициализацию экземпляра синглтона туда, но в конце мы потеряли в проблемах параллелизма: было либо несколько экземпляров Hudson, либо приложение само заблокировалось. Мы пытались переместить фрагменты кода, но зависимости не позволили бы нам сделать это.
Успех
Размышляя о нашей ошибке, я пришел к выводу, что проблема заключалась в том, что Hudson.getInstance () вызывается (много раз) уже во время выполнения конструктора Hudson объектами, которые там используются, и потоки начинаются оттуда. Это, конечно, отвратительная практика — получать доступ к полусгоревшему экземпляру до его полной инициализации. Решение тогда простое: чтобы иметь возможность инициализировать одноэлементное поле вне конструктора, мы должны удалить все вызовы getInstance из его контекста.
Шаги хорошо видны из соответствующих коммитов GitHub. Резюме:
1. Я использовал рефакторинг «Представляем фабрику» на конструкторе
2. Я изменил ProxyConfiguration, чтобы не использовать getInstance, а ожидать, что корневой каталог будет установлен до его первого использования.
3. Я переместил код, который не нужно было запускать из конструктора, в новый метод фабрики — это привело к некоторому, надеюсь незначительному, переупорядочению кода
4. Наконец, я также перенес инициализацию экземпляра в фабричный метод
Я не могу быть на 100% уверен, что полученный код имеет такую же семантику, насколько это важно, поскольку мне пришлось сделать несколько изменений вне безопасного автоматизированного рефакторинга, и не было никаких полезных тестов, кроме попыток запустить приложение (и , как это часто бывает с унаследованными приложениями, было невозможно создать их заранее).
Реорганизованный код пока не дает большой добавленной стоимости, но это хорошее начало для дальнейших рефакторингов (которые у меня не будет времени попробовать :-(), он избавился от оскорбительного использования экземпляра во время его работы. Создан и код конструктора проще и лучше. Упражнение заняло у меня около четырех pomodoros , то есть чуть меньше двух часов.
Если бы у меня было время, я бы продолжил извлекать интерфейс из Hudson, перенося его несвязанные обязанности на собственные классы (возможно, сохраняя методы в Hudson для обратной совместимости и делегируя этим объектам), и я мог бы даже использовать некоторую магию AOP чтобы получить более чистый код при сохранении бинарной совместимости (как на самом деле уже делают Хадсон / Дженкинс).
Попробуйте сами!
Настроить
Получить код
Получите код как .zip или через git:
1
2
3
|
cd coding-dojo git checkout -b mybranch INITIAL |
Скомпилируйте код
как описано в README додзё .
Запустите Дженкинс / Хадсон
1
2
3
|
cd coding-dojo /2011-04-26-refactoring_hudson/ cd maven-plugin; mvn install ; cd .. # a necessary dependency cd hudson /war ; mvn hudson-dev:run |
и перейдите по адресу http: // localhost: 8080 / (Jetty должна автоматически выбирать изменения для файлов классов).
Дальнейшие рефакторинги
Если вы любитель приключений, вы можете попытаться улучшить код, разделив индивидуальные обязанности класса богов. Я бы поступил так:
- Извлеките интерфейс из Hudson и используйте его везде, где это возможно
- Переместите связанные методы и поля в собственные (вложенные) классы, оригинальные методы Хадсона просто делегируют им (рефакторинг метода перемещения должен быть полезен); например:
— Управление расширениями и дескрипторами
— Аутентификация и авторизация
— управление кластером
— Функциональность уровня приложения (методы контроля, такие как перезапуск, обновление конфигураций, управление прослушивателями сокетов)
-UI контроллер (для этого потребуется переконфигурация сшивателя)
- Преобразуйте вложенные классы в классы верхнего уровня
- Предоставить способ получения экземпляров классов без Hudson, например, в виде синглетонов
- По возможности используйте отдельные классы вместо Hudson, чтобы другие классы зависели только от функциональности, которая им действительно нужна, а не от всего Hudson
Узнав о Дженкинс / Хадсон
Если вы хотите понять режим о том, что делает Хадсон и как он работает, вы можете проверить:
- Архитектура Гудзона и, возможно, продолжить с
- Здание Гудзон
- Введение в пользовательский интерфейс Stapler (его ключевой особенностью является то, что он умно отображает URL-адреса в иерархии объектов [и просматривает файлы и методы действий]), возможно, проверьте также ссылку на Stapler
Sidenote: Хадсон против Дженкинса
Когда-то существовал сервер непрерывной интеграции под названием Hudson, но после смерти его покровителя Sun он оказался в руках человека по имени Oracle. Он не был очень хорош в общении, и никто не знал, что он задумал, поэтому, когда он начал вести себя немного странно — или, по крайней мере, так понимали его друзья Хадсона — те, кто беспокоился о будущем Хадсона (включая большинство людей, изначально работавших в проект) сделал свой клон и назвал его Дженкинс, что является еще одним популярным именем для дворецких. Так что теперь у нас есть Хадсон при поддержке Oracle и ребята из Sonatype и Jenkins при поддержке яркого сообщества. Это упражнение основано на исходном коде Jenkins, но для поддержания низкого уровня путаницы я часто называю его Hudson, поскольку так называются пакет и основной класс.
Вывод
Рефакторинг устаревшего кода всегда оказывается более сложным и трудоемким, чем вы ожидаете. Важно следовать какому-либо методу, например, методу Микадо, который помогает вам вести глобальный обзор того, куда вы хотите пойти и куда вы находитесь, и регулярно обдумывать, что и почему вы делаете, чтобы не потеряться в серия исправить проблему — новые проблемы обнаружены шаги. Важно понимать, когда нужно сдаться и попробовать другой подход. Также очень трудно или невозможно написать тесты для изменений, поэтому вы должны быть очень осторожны (максимально использовать безопасные, автоматизированные рефакторинги и действовать небольшими шагами), но страх не должен мешать вам пытаться спасти код от распада.
Ссылка: Что я узнал от (почти) неудачи в рефакторинге Хадсона от нашего партнера по JCG в блоге Holy Java .
Статьи по Теме:
- Java Code Geeks Andygene Web Archetype
- Почему автоматизированные тесты ускоряют вашу разработку
- Качество кода имеет значение для клиентов. Много.
- Использование FindBugs для создания значительно меньшего количества ошибочного кода
- Рекомендации по разработке гибкого программного обеспечения для пользователей и новых пользователей