Вопрос, который мне задают чаще всего, — как разговаривать о рефакторинге с руководителем?Мартин Фаулер, «Рефакторинг. Улучшение существующего кода»
В таких случаях я даю несколько спорный совет: не говорите ему ничего!
Устаревание кода, трудности с поддержкой, непредсказуемые баги — эти термины один за другим появляются в жизни разработчика по мере разработки продукта. И если первое — это скорее интересы разработчика, то последнее — это прямая проблема бизнеса.
В этой статье я хочу поделиться опытом переписывания большого проекта и как бонус привести пару кусков кода, которые помогли нам и, надеюсь, помогут вам начать этот интересный путь.
Разбор полетов
Проблемы
Они обычно начинаются по известному сценарию:
- Прибегает начальник с воплями «У нас ничего не работает, главный клиент под угрозой!»;
- или менеджер с просьбой прикрутить нереализуемую фишку;
- реже мы, разработчики, настолько устаем копаться в
говне«легаси»-коде, что решаем переписать всё.
И обычно это заканчивается всеобщим негодованием и разладом, потому что фишка нужна срочно, клиенты тоже ждать не могут, а из-за печального наследия команда стремится разбежаться. Ситуацию портит отсутствие «денег на рефакторинг» (бездействие команды в понятиях бизнеса)
Насчет последнего пункта нужно добавить, что ситуацию с новым человеком в команде, который рвется всё переписать, я не рассматриваю, однако он может запросто аргументировать описанный подход для развития проекта.
Задачи
- Перевести проект на современную архитектуру
- Обеспечить минимальные затраты на рефакторинг
Принципиальная схема реализации
Наш проект был изначально написан на Kohana, переписывали мы его на Symfony2, поэтому все примеры приведены в контексте этих систем. Однако, данный подход можно применять с любыми фреймворками. Необходимое требование — единая точка входа в приложение.
Изначально приложение обрабатывает запросы пользователя через точку входа «app_kohana.php»
Мы будем оборачивать начальную точку входа в новой системе, организовывая своеобразный «прокси».
Рефакторинг
Контроллер — обертка для старой системы
Идея довольно проста и заключается в следующем:
- Разворачиваем параллельно две системы (kohana + symfony)
- Меняем точку входа на новую (symfony)
- Организуем универсальный контроллер, который по умолчанию будет пробрасывать все запросы в старую систему
И если с первыми двумя пунктами проблем возникнуть не должно, то третий представляет интерес, потому как в нем могут обнаружиться подводные камни.
Первое, что приходит в голову — обернуть инклюд в ob_start. Так и сделаем:
class PassThroughController extends Symfony\Bundle\FrameworkBundle\Controller\Controller {
public function kohanaAction()
{
ob_start();
$kohanaPath = $this->container->getParameter('kernel.root_dir') . '/../app_kohana.php';
include $kohanaPath;
$response = ob_get_clean();
return new Response($response);
}
}
application.passthrough_kohana:
path: /{slug}
defaults:
_controller: ApplicationBundle:PassThrough:kohana
requirements:
slug: .*
В таком формате система уже работает, но спустя какое-то время прилетает первый баг. Например, некорректная обработка ajax-ошибок. Или на сайте ошибки отдаются с кодом 200 вместо 404.
Тут мы понимаем, что буфер проглатывает заголовки, поэтому их нужно обрабатывать явным образом
class PassThroughController extends Symfony\Bundle\FrameworkBundle\Controller\Controller {
public function kohanaAction()
{
ob_start();
$kohanaPath = $this->container->getParameter('kernel.root_dir') . '/../app_kohana.php';
include $kohanaPath;
$headers = headers_list();
$code = http_response_code();
$response = ob_get_clean();
return new Response($response, $code, $headers);
}
}
После этого полёт нормальный.
Проблемы старой системы, влияющие на функционирование новой
exit()
У нас в системе нашлись места, где в конце работы контроллера радостно вызывался exit(). Это практикуется, например, в Yii (CApplication::end()). Особой головной боли это не доставляет до тех пор, пока не начинаешь использовать событийную модель в новой системе и обрабатывать события, случающиеся после выполнения контроллера. Самый яркий пример — Symfony Profiler, который прекращает работать для запросов с exit'ом.
Данный случай нужно иметь в виду и при необходимости предпринимать соответствующие меры.
ob_end_*()
Необдуманное использование функций ob_end легко может поломать работу новой системы, очистив буфер нового прокси-контроллера. Следует так же иметь в виду.
Kohana_Controller_Template::$auto_render
Переменная отвечает за автоматическую отрисовку полученных из контроллера данных в глобальном шаблоне (может сильно зависеть от используемого шаблонизатора). Во время адаптации новой системы это может сэкономить время на отладку в местах, где, например, json выводится простым echo $json; exit();. Контроллер примет примерно следующий вид:
$this->auto_render = false;
echo $json;
return;
О чем еще стоит позаботиться
Описанные выше точки входа — это идеальная ситуация. У нас изначально точка входа была app.php и требовалось, чтобы после рефакторинга она же и осталась (переконфигурирование многочисленных серверов выглядело бесперспективным). Выбран был следующий алгоритм:
- Переименовываем app.php в app_kohana.php
- Точку входа симфони размещаем в app.php
- Profit
И все, казалось бы, завелось, кроме консольных команд, которые в кохане запускались через тот же файл. Поэтому в начале нового app.php родился следующий костылик для обратной совместимости:
if (PHP_SAPI == 'cli') {
include 'app_kohana.php';
return;
}
Жизнь после рефакторинга
Новые контроллеры
Все новые контроллеры мы стараемся писать в symfony. Разделение происходит на уровне роутинга, перед «универсальным» маршрутом дописывается нужный, и Kohana дальше не загружается. Пока мы пишем в новой системе только ajax-контроллеры, поэтому вопрос с переиспользованием шаблонов (Twig) остается открытым.
БД и Конфигурация
Для доступа к БД были сгенерированы модели из текущей базы стандартными методами Doctrine. В репозитории по мере необходимости добавляются новые методы работы с БД. Однако, конфигурация подключения к БД используется существующая из коханы. Для этого написан конфигурационный файл, который подтягивает данные из конфига коханы и преобразует их в параметры конфигурации симфони. Логика поиска конфига в зависимости от платформы, увы, продублирована, чтобы не подключать классы коханы в новой системе.
/** @var \Symfony\Component\DependencyInjection\ContainerBuilder $container */
$kohanaDatabaseConfig = [];
$kohanaConfigPath = $container->getParameter('kernel.root_dir') . '/config';
if (!defined('SYSPATH')) {
define('SYSPATH', realpath($container->getParameter('kernel.root_dir') . '/../vendor/kohana/core') . '/');
}
$mainConfig = $kohanaConfigPath . '/database.php';
if (file_exists($mainConfig)) {
$kohanaDatabaseConfig = include $mainConfig;
}
if (isset($_SERVER['PLATFORM'])) {
$kohanaEnvConfig = $kohanaConfigPath . '/' . $_SERVER['PLATFORM'] . '/database.php';
if (file_exists($kohanaEnvConfig)) {
$kohanaDatabaseConfig = array_merge($kohanaDatabaseConfig, include $kohanaEnvConfig);
}
}
if (empty($kohanaDatabaseConfig['default'])) {
throw new \Symfony\Component\Filesystem\Exception\FileNotFoundException('Could not load database config');
}
$dbParams = $kohanaDatabaseConfig['default'];
$container->getParameterBag()->add([
'database_driver' => 'pdo_mysql',
'database_host' => $dbParams['connection']['hostname'],
'database_port' => null,
'database_name' => $dbParams['connection']['database'],
'database_user' => $dbParams['connection']['username'],
'database_password' => $dbParams['connection']['password'],
]);
Подключается конфиг стандартным способом
class ApplicationExtension extends Symfony\Component\HttpKernel\DependencyInjection\Extension {
public function load(array $configs, ContainerBuilder $container) {
$loader = new Loader\PhpFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('kohana.php');
}
}
Как продолжать: вынесение функционала в сервисы
Дальнейшей движение из коханы в симфони очень хорошо укладывается в вынесение функционала в сервисы симфони и использовании их в старой системе через DI-контейнер. Так сложилось, что DI-компонент мы начали использовать до подключения симфони в проект, поэтому этот процесс прошел довольно гладко, но ничто не мешает делать это с нуля. Основной задачей будет прокинуть DI-контейнер из симфони в кохану. Мы сделали это в кохана-стиле через статическое свойство, в другом фреймворке можно найти соответствующий подход.
class Kohana extends Kohana_Core {
/**
* @var Symfony\Component\DependencyInjection\ContainerBuilder
*/
public static $di;
}
А дальше нужно провернуть еще пару махинаций, чтобы положить в этой свойство DI-контейнер между инициализацией коханы и выполнением кода контроллера. Для этого разделим наш файл инициализации app_kohana.php на две части, выделив непосредственно инициализацию системы и сам запуск контроллера.
/** app_kohana_init.php */
// тут инициализация фреймворка, включая системные константы и bootstrap
/** app_kohana_run.php */
echo Request::factory(TRUE, array(), FALSE)
->execute()
->send_headers(TRUE)
->body();
/** app_kohana.php */
include 'app_kohana_init.php';
include 'app_kohana_run.php';
Модифицируем наш контроллер, проделывая похожие с app_kohana.php операции, но добавляя между инклюдами проброс контейнера
public function kohanaAction() {
ob_start();
$kohanaPath = $this->container->getParameter('kernel.root_dir') . '/..';
include $kohanaPath . '/app_kohana_init.php';
\Kohana::$di = $this->container;
include $kohanaPath . '/app_kohana_run.php';
$headers = headers_list();
$code = http_response_code();
$response = ob_get_clean();
return new Response($response, $code, $headers);
}
После этого мы в старой системе можем использовать DI-контейнер и все объявленные в новой системе сервисы, включая EntityManager и новые модели доктрины.
Напоследок
Плюсы реализации
- Мы сделали первый шаг для дальнейшего развития системы.
- Новая система независима от старой. Весь новый код работает без участия старого
- Минимум потраченного времени
Минусы реализации
- Дополнительные накладные ресурсы на «обертку» во время работы со старой частью системы. Однако, по сравнению с задержками в старой системе, оверхедом (как по памяти, так и по процессору) можно пренебречь.
- Новая система независима от старой. Мы не можем использовать старый код в новой, но это скорее плюс, раз уж мы решились переписывать.
- Приходится поддерживать модели в двух местах.
Спасибо, что дочитали до конца, желаю успехов в рефакторинге, смахните накопившуюся пыль со старого кода!
И простите за ужасные шрифты на диаграммах :(
Комментарии (37)
neolink
30.03.2015 20:02+7Мне кажется, что если в коде есть переменные Костыль1 и Костыль2 да еще и с нецензурными префиксами в имени, то без изменения команды никакими рефакторингами проблемы не решить
nitso Автор
30.03.2015 22:57-1Любопытно услышать, почему.
Я склонен причислять программирование к творческим занятиям, а куда там до творчества без эмоций?
Даже если посмотреть с практической точки зрения, этот стыд хочется выпилить быстрее, чем кучку TODO и FIXME — замечательная мотивация.neolink
30.03.2015 23:53+3театральные постановки с матом я тоже не понимаю, если уж творишь, то старайся не натворить. И способов передать эмоциональное состояние есть предостаточно, а если не владеешь минимальным их набором то может лучше и не браться?
В программировании на самом деле не должны быть места эмоциям, логика тут правит балом. И если возможно объяснить почему не стал рефакторить можно (сроки и т.п.), то такой нейминг говорит о культуре человека и у него не может быть оправданий.
garex
30.03.2015 20:37+3А просто rewrite-ы на уровне веб-сервера не оно? Работать будет в стопицот раз быстрее, без оверхеда и надежнее. Все равно по началу маленькие куски нового фреймворка юзаются.
limitium
30.03.2015 22:09Вопрос, который мне задают чаще всего, — как разговаривать о рефакторинге с руководителем?
В таких случаях я даю несколько спорный совет: не говорите ему ничего!
Ага молча делать его за свой счет — это надо сильно любить свой проект и работу.
Когда будет вторая часть «Безболезненный рефакторинг фронтенда»?
TheShock
31.03.2015 14:38+4Ага молча делать его за свой счет — это надо сильно любить свой проект и работу.
Просто в каждую задачу закладывать время на рефакторинг
maximw
31.03.2015 15:32Это тоже не совсем правильно.
Работодатель должен знать, чем занимаются программисты, как минимум из соображений честности.
И однажды можно услышать совершенно справедливый вопрос от недоинформированного работодателя «А какого, собственно, у вас время уходит в разы больше, чем могли бы сделать другие программисты, или чем вы делали раньше?»TheShock
31.03.2015 16:06+2Я на такое б ответил так: «Если вы наняли меня — вы хотели качества. Вы можете нанять другого программиста.»
А вообще если всегда соблюдать такое правило, то не будешь сильно выбиваться за сроки других программистов за счет значительно меньшего технического долга.
И я говорю про то, что в рамках любой задачи необходим рефакторинг (не глобальный и всего проекта, а локальный). Это абсолютно чесно. Вставляете комментарии к статье в бложику — рефакторите связанную с этим часть, а не впихиваете абы как.
m00t
31.03.2015 16:19Бизнес нанимает программиста, чтобы заработать денег. Ему не то чтобы сильно важно, что там под капотом. Поэтому когда вы придете к нему и скажете «вы хотели качества», он вас не поймет, и вы поругаетесь. Единственная причина рефакторить — сделать то же самое дешевле, чем это было бы без рефакторинга. Иначе рефакторинг бесполезен.
nitso Автор
31.03.2015 16:58Во всем должна быть мера.
Бизнес обязан понимать разницу между прототипом и продуктом. У этих двух состояний есть одна цель — получать прибыль. Но если прототип ориентирован на краткосрочный период, то продукт уже озадачен долговременной поддержкой.
Задача рефакторинга (в понятиях бизнеса) — как раз сделать дешевой поддержку. На начальном этапе поддержка не так важна, и есть риск не выбраться из прототипа в условиях быстро меняющихся потребностей.
Ниже отличный коммент про менеджмент vs программисты.
Но по моему опыту все скатывается до «угадали программисты, чего захочет бизнес, или нет». Хорошо, если есть ответственный за это человек в команде, а когда программирование ведется в отрыве от бизнеса, жди беды, если не угадал.m00t
31.03.2015 17:22+1Я вообще сторонник подхода, чтобы не разделять прототип и продукт. Я убежден, что прототип постепенно должен становиться продуктом по мере работы над ним. Самые кривые вещи должны постепенно рефакториться, когда они начинают доставлять неудобства. И таким образом, кривой тяп-ляп прототип постепенно должен превращаться в нормальный продукт. Мне кажется, такая эвристика в большинстве случаев подходит, если надо что-то побыструхе вывести на рынок и протестировать идею. Потому что будем честны, много ли прототипов удается переписать с нуля? Обычно этот шаг откладывается до одного большого «рефакторинга», который никогда не наступает.
TheShock
31.03.2015 17:01+1Единственная причина рефакторить — сделать то же самое дешевле, чем это было бы без рефакторинга
Вы так недалекоглядны. Не всегда в бизнесе преследуют только краткосрочные цели. В среднесрочной и выше перспективе подход «тяп-ляп» является убыточным для бизнеса. Потому очень редко программисты-быдлокодеры получают большие деньги.m00t
31.03.2015 17:06Вы так недалекоглядны
Ну офигеть, пошел переход на личности и ярлыки.
Не всегда в бизнесе преследуют только краткосрочные цели
Эммм. А я где говорил про краткосрочные vs долгосрочные? Я говорил — дешевле. Решить, дешевле на каком промежутке это должно быть — задача бизнеса, а не программиста.
Davert
31.03.2015 02:31+6И ни слова о тестах. А без тестов рефакторить это очень и очень плохая затея.
В данном случае я не имею ввиду юнит-тесты. Как по мне, стоил покрыть старый код приемочными тестами. Чтобы знать, что всё старое будет работать на новом кодеnitso Автор
31.03.2015 02:41+2Абсолютно согласен, это стоило упомянуть в статье, тем более, что с тестами всё хорошо. Без них этот материал не родился бы.
m00t
31.03.2015 12:25+5Вы описали очень хороший способ перехода с одного фреймворка на другой. Но по моему опыту (я писал раньше на Kohana, и пишу теперь на Symfony), степень говнокода — это особенность системы, а не фреймворка, на которой все написано. Если у вас были тяжелые и непонятные контроллеры в Kohana, вас будут ждать такие же в Symfony, и поддерживать это дело на Symfony будет не обязательно проще (а чаще — сложнее, т.к. вы начнете тратить уйму времени на борьбу с фреймворком там, где раньше вы просто делали class Kohana extends Kohana_Core и вворачивали любой костыль). Я бы посоветовал вам бросить ресурсы не на переход от одного фреймворка на другой, а на рефакторинг кода в рамках одного фреймворка. Возможно, имеет смысл начать юзать высококачественные компоненты (Doctrine, symfony/form) в вашем проекте на Kohana. Или начать выносить бизнес-логику из контроллеров в отдельный слой. И это может дать намного больше профита с меньшими затратами, чем переписывать все на другом фреймворке.
nitso Автор
31.03.2015 12:56+3У нас всё примерно и происходило по описанному вами сценарию. Началось с подключения DI-контейнера (вынесение бизнес-логики в отдельный слой), логгера (monolog) и мейлера (swiftmailer), потом пришел eventdispatcher и т.д. К моменту, когда мы подумали, что хотим заменить KohanaORM на Doctrine, мы уже использовали бОльшую часть symfony, поэтому решение о переходе целиком на symfony далось нам легко.
Сейчас старый код плавно мигрирует из kohana в symfony, не доставляя головной боли. На переписывание ресурсы практически не тратятся.
Dimchansky
31.03.2015 12:30Оффтоп. Напомните название сервиса, где можно делать такие «фото» кода, как на первой картинке.
m00t
31.03.2015 12:48+1Немного оффтоп.
Я абслютно согласен: рефакторинг — не дело менеджеров или бизнеса. Команда сама должна решать, когда и сколько делать рефакторинг. Единственный критерий тут, и единственная причина делать рефакторинг: как быстрее и дешевле (при заданном бизнесом уровне надежности) сделать для бизнеса то, что ему надо. И тут уже это могут знать только технические специалисты.
Другое дело, что часто менеджемент не доверяет техническим специалистам в этом вопросе. И, кстати, правильно делает =). Потому что очень часто программисты подменяют «быстрее и дешевле» на «нам интереснее». И это часто можно видеть не только от новичков, но и от очень серьезных девелоперов. Когда за счет бизнеса идет обучение новым технологиям или просто развлечение со всякими крутыми штуками.
Отсюда у меня такой вывод: в извечной войне «менеджеры против рефакторинга» программисты должны для начала учиться работать с менеджементом и с бизнесом по одну сторону баррикад. Постепенно работать над этим доверием, когда если программист говорит «нам надо делать А», это значит, что программист совершенно уверен, что для бизнеса «А» будет полезно, а не это просто его прихоть изучить технологию «А».
Archon
31.03.2015 13:52+2Для этого в команде и должен существовать IT-директор (CIO), гибрид между бизнес-руководителем и технарём, которому можно объяснить, что после того, как мы переписали 5% кода на технологию А, на том же железе бенчмарки стали летать на 70% быстрее, и который сможет выделить объективно требуемые для рефакторинга ресурсы.
Если его заменить, скажем, руководителем отдела разработки (старшим технарём), получится фигня, поскольку ему тоже может быть что-то по приколу покрутить, а оптимизация бизнеса — вообще не его головная боль.m00t
31.03.2015 14:04Я думаю, что в этом вся и проблема, что многие думают «оптимизация бизнеса — вообще не его головная боль».
Я считаю, что подход «девелоперы беспокоятся про оптимизацию бизнеса» может применяться на абсолютно любого размера проектах, начиная от ситуации, где проект делает 1 девелопер, и для этого не должно быть никаких требований по наличию или отсутствию каких-либо ролей. У нас только так не принято пока что, к сожалению. Вот и страдаем.Archon
01.04.2015 10:00+11) Не каждый разработчик вообще способен думать об оптимизации затрат.
2) Не каждый из тех, кто способен, хочет об этом думать.
3) Не каждому из тех, кто хочет, готовы за это доплачивать.m00t
01.04.2015 22:43-1Пункты 2-3 самые прикольные у вас. Получается, программисту надо доплачивать за то, что он будет думать во время работы? И не каждый программист хочет думать во время работы? WAT??? Я всегда думал, что программист должен делать все, что в его силах в свое рабочее время, чтобы сделать работу лучше. Оказывается, нет?
Archon
02.04.2015 08:23+3Нет. Любой сотрудник, включая программиста, должен выполнять свою работу в объёме должностных обязанностей, оговорённых в собеседовании и трудовом договоре. Если же должностные обязанности существенно расширились в процессе работы, это хороший повод поставить вопрос либо о соответствующем увеличении оплаты, либо о снижении объёма работы.
Конечно, каждая компания мечтает о фанатиках, которые будут круглосуточно заботиться об их бизнесе, да ещё и за цену обычного программиста, транслирующего ТЗ в код от звонка до звонка. Но таких людей мало, да и однажды обжёгшись в одной компании, в следующей они так себя вести уже не будут.
koresar
01.04.2015 03:04Из моего опыта.
Проблема рефакторинга/легаси кода исчезает насовсем при использовании архитектуры микросервисов.
Должен сказать, что появляется проблема оркестрирования сервисов, но она легко решаема.
Corpsee
Поправочка: в Yii (если по-уму) не практикуется exit(), а практикуется Yii::app()->end(). Это не одно и то же.
nitso Автор
К сожалению, в самом методе end как раз используется exit (по умолчанию, если не передавать дополнительных флагов).
Как в первой версии, так и во второй
Corpsee
Тем не менее, end позволяет обработать прекращение работы перед exit, что сильно лучше, чем просто убить скрипт никому ничего не сообщив. Вы в статье про событийную модель упомянули, вот как раз end — это и есть событие завершения работы.
Corpsee
Ну так вызывайте с нужными аргументами не по умолчанию или переопределите метод. Все таки Yii не заставляет вас использовать exit, это от вас зависит.
annenkov
убрали бы вы лучше это упоминание про YII в таком контексте, ибо необоснованно — по приведенной вами же ссылки видно, что в Yii::app()->end() вызывается обработчик события прежде чем сработает exit().
nitso Автор
Наличие событий в Yii никак не избавляет от описанной проблемы с exit.
Цепочка вызовов прерывается в любом случае для описанной схемы (symfony (init) -> Yii (init-controller-end) -> symfony (end)). Цель упоминания в статье — обратить внимание на эту особенность, а не кинуть камень в огород Yii.