Привет, сегодня я хочу поговорить об ужасной кодовой базе, с которой вы, скорее всего, прямо сейчас имеете дело. Есть проект и вы нужны, чтобы добавлять новые фичи и фиксить баги. Но вы открываете IDЕ, делаете пул репозитория с проектом — и хочется плакать. Кажется, что с этим кодом невозможно работать.
Картина, конечно, немного удручающая, но… давайте отбросим эмоции. И посмотрим, что можно быстро предпринять, чтобы облегчить страдания.
Почему мы чувствуем себя несчастными, работая с легаси-кодом?
Если порассуждать, легаси не так уж и плох: это значит, что проект жив, он приносит деньги и в нем есть какая-то польза. Конечно, круто, когда мы сами с нуля выбираем язык, базу данных, систему обмена сообщениями, проектируем архитектуру — но няшный стартап может так никогда и не попасть в прод. А легаси — уже радует пользователей.
Но с точки зрения старшего разработчика часто все выглядит так:
приложение очень хрупкое: если фиксим где-то баг, в ответ прилетают два новых;
есть regressions bugs — они как вампиры, которых невозможно убить, каждый релиз появляются снова и снова;
новые фичи занимают все больше времени: откладываются релизы, давят дедлайны, вы начинаете сознательно увеличивать время на планировании.
Причиной этих бед, как правило, является огромный объем технического долга — и отношение к нему в команде. Тесты, рефакторинги требуют времени. Конечно, когда важнее деливерить фичи, хочется на это забить. И я думаю, мы сами допустили подобное в индустрии.
У меня есть одна история. Приходит менеджмент, говорит, что нам нужна эта фича, причем «еще вчера». Мы работаем изо всех сил, релизим как можно быстрее. Затем говорим менеджменту: «Теперь нам нужна неделя, чтобы отрефакторить, написать тесты». На что менеджер отвечает: «Что? Вы этого не сделали?».
Когда вас просят сделать что-то, от вас ожидают что-то готовое. А если фича нужна настолько срочно, что к ней даже не нужна «пожарная сигнализация» в виде тестов, это должно быть оговорено в самом начале. И наша задача как разработчиков донести это до бизнеса. Я хочу поделиться подходом, который мы применили в команде мобильного бэкенда Skyeng — это путь инкрементального улучшения легаси-проекта.
Что будет, если ничего не делать (и почему переписать не всегда выход)
Каюсь, как-то раз я работал в компании, где использовался подход «нет времени ничего менять, давайте быстро пофиксим». Мы продолжали добавлять фичи, плодить (и фиксить) всё больше багов. Мы лечили не болезнь, а лишь ее симптомы. Я на практике столкнулся с «эффектом разбитых окон». Начав делать фиксы по принципу «лишь бы просто работало» и заметив, что, в принципе, и новичкам и старичкам в команде всё равно, мы начали жить по принципу: быстрый фикс, копи-паста, костыль — сделали, зарелизили, забыли.
Становилось всё хуже и хуже. Всё это привело к серьезному стрессу и потере мотивации. В конце концов, не дожидаясь, когда мы подойдём к моменту, что всё это станет не поддерживаемо, я ушел.
Есть другая крайность. Мы видим старую систему. Думаем: «Если тот криворукий, кто написал этот ужас, смог заставить всё работать, то наверняка это просто. Давайте перепишем всё уже по-нормальному. Сделаем красиво». Но это ловушка. Легаси-код уже пережил много деплоев и изменений требований. Начиная с начала, мы думать не думаем и знать не знаем о всех тех трудностях, которые пережил тот код. Да, мы пытаемся делать предположения, но зачастую они неверны. Почему так?
Есть старая команда, которая поддерживает легаси-проект, и новая — у нее обычно нет большой накопленной экспертизы ни в домене, ни в легаси-коде. Новая команда будет делать слишком много предположений о системе. И не всегда угадает.
Я не хочу сказать, что это не работает. У меня есть позитивный опыт переписывания легаси-системы: но, во-первых, тот, кто писал старый проект, и переписывал его. Во-вторых, бизнес дал добро, что на определенный промежуток времени мы замораживаем систему — ничего больше не добавляем, не фиксим без крайней необходимости. Плюс у нас под рукой всегда был доменный эксперт. Но у меня до сих пор есть ощущение, что нам повезло и это было скорее исключение, подтверждающее правило.
Шаг первый: пишем smoke-тесты, которые скажут, что приложение хотя бы живое
Иметь тесты действительно важно для бизнеса. И если тестов до вас не было, а вы решили что-то улучшать, начните с них. Возможно, на данном этапе вам не нужны «правильные тесты» — юнит, приемочные и т.д. Начните с «просто тестов», которые будут рассматривать приложение как черный ящик, — и покройте хотя бы критические эндпоинты. Например, убедитесь, что после рефакторинга регистрации приложение хотя бы не пятисотит.
Что использовать? Нам понравился Codeception — у него приятный интерфейс, можно быстро накидать smoke-тесты на наиболее критичные эндпоинты, сами тесты выходят лаконичными и понятными. А еще у него такой выразительный API и тест можно читать прямо как user story.
Что покрывать? Правило простое: покрываем тот функционал, который отвечает за бизнес-домен. В нашем случае это были тренировки слов — мы покрыли smoke-тестами его. Например, у нас был эндпоинт, который отдавал выученные пользователем значения слов.
/**
* @see \WordsBundle\Controller\Api\GetLearnedMeanings\Controller::get()
*/
final class MeaningIdsFromLearnedWordsWereReceivedCest
{
private const URL_PATH = '/api/for-mobile/meanings/learned';
public function responseContainsOkCode(SmokeTester $I): void
{
}
}
Мы явно прописали эндпоинт, который будем тестировать, и через аннотацию @see сделали указание на контроллер. Это удобно тем, что по клику в IDE можно сразу перейти в контроллер. И наоборот, по клику на экшен контроллера можно перейти в тест.
Сам тест был максимально прост: подготавливаем данные, сохраняем в базе выученное пользователем слово, авторизуемся под этим юзером, делаем get-запрос к API и проверяем, что получили 200.
public function responseContainsOkCode(SmokeTester $I): void
{
$I->amBearerAuthenticated(Identity::USER);
$I->sendGET($I->getApiUrl(self::URL_PATH));
$I->seeResponseCodeIs(Response::HTTP_OK);
}
Такой тест — это самое простое, что можно сделать, но это уже дает подушку безопасности. Теперь мы могли быть уверены: после рефакторинга приложение не запятисотит.
Но потом нам захотелось большего, и мы допилили тест: стали дополнительно проверять схему ответа и что в нем вернулось действительно то, что нужно. В нашем случае — выученное слово.
public function responseContainsLearnedMeaningIds(SmokeTester $I): void
{
$learnedWord = $I->have(
UserWord::class,
['isKnown' => true, 'userId' => $I->getUserId(Identity::USER)]
);
$I->amBearerAuthenticated(Identity::USER);
$I->sendGET($I->getApiUrl(self::URL_PATH));
$I->seeResponseCodeIs(Response::HTTP_OK);
$I->seeResponseJsonMatchesJsonPath('$.meaningIds');
$I->seeResponseContainsJson(
['meaningIds' => [$learnedWord->getMeaningId()]]
);
Так постепенно мы покрыли все критичные части нашего приложения — и могли перейти непосредственно к коду.
Про тесты
Недостаточно просто написать тесты — важно их поддерживать максимально наглядными, понятными и легко расширяемыми. Нужно относиться к коду тестов так же трепетно, как мы относимся к коду приложения, которое тестируем.
Шаг второй: удаляем неиспользуемый код
В проекте не нужен код, который «потом понадобится». Потому что нас есть git, который уже его запомнил. Надо будет — восстановим.
Как понять, что удалять, а что нет. Довольно легко разобраться с каким-то доменным или инфраструктурным кодом. Совсем другое дело — API и эндпоинты. Чтобы разобраться с ними, можно заглянуть в NewRelic и проекты на гитхабе. Но лучше прямо сходить к командам и поспрашивать — может статься, что у вас есть какой-то эндпоинт, из которого аналитика раз в год выкачивает важные данные. И будет очень некрасиво удалить его, даже если по всем признакам его никто не вызывал уже почти год.
Шаг третий: делаем слой вывода
Очень часто в легаси-приложениях либо сущности торчат наружу, либо где-то в контроллере собирается и отдается массив. Это плохо — скорее всего, у вас нарушена инкапсуляция и у сущности есть публичные свойства. Такое тяжело рефакторить: клиенты в ответе уже ожидают определенные поля, и если мы захотим переименовать или сделать приватным какое-то свойство, то даже обнаружив и пофиксив все использования, мы имеем шанс что-то сломать и разбираться с фронтом.
Например, у нас есть контроллер, который возвращает баланс пользователя. В этом примере мы просто достаём из репозитория сущность и как есть отдаём наружу — то есть как только мы порефакторим, API у клиента поменяется.
final class Controller
{
private Repository $repository;
public function __construct(Repository $repository)
{
$this->repository = $repository;
}
public function getBalance(int $userId): View
{
$balance = $this->repository->get($userId);
return View::create($balance);
}
}
Чтобы решить проблему, можно вместо этого в ответах всегда использовать простую DTO — пускай у нее будут только те поля, которые нужны в ответе клиентам. Добавим маппинг из сущности и уже в контроллере это вернем.
final class Balance
{
public int $userId;
public string $amount;
public string $currency;
public function __construct(int $userId, string $amount, string $currency)
{
$this->userId = $userId;
$this->amount = $amount;
$this->currency = $currency;
}
}
Всё, больше слой вывода никак не привязан к доменному коду, сам код стал более поддерживаемым.
final class Controller
{
private Repository $repository;
public function __construct(Repository $repository)
{
$this->repository = $repository;
}
public function getBalance(int $userId): View
{
$balance = $this->repository->get($userId);
return View::create(Balance::fromEntity($balance));
}
}
Можно спокойно начинать улучшать и рефакторить, не боясь сломать обратную совместимость API.
Шаг четвертый: статический анализ кода
В современном PHP есть strict types, но даже с ними можно по-прежнему менять значение переменной внутри самого метода или функции:
declare(strict_types=1);
function find(int $id): Model
{
$id = '' . $id;
/*
* This is perfectly allowed in PHP
* `$id` is a string now.
*/
// …
}
find('1'); // This would trigger a TypeError.
find(1); // This would be fine.А так как типы проверяются в runtime, проверки могут зафейлится во время выполнения программы. Да, всегда лучше упасть, чем словить проблем из-за того, что строка внезапно превратилась в число (причем не то число, которое мы бы ожидали увидеть). Но иногда хочется просто не падать в runtime. Мы можем выполнить проверки до выполнения кода с помощью дополнительных инструментов: Psalm, PHPstan, Phan и так далее.
Мы в проекте выбрали Psalm. Возможно, у вас возник вопрос: зачем тащить в легаси-проект статический анализ, если мы и без него знаем, что код там не очень? Он поможет проверить неочевидные вещи. В легаси-проектах часто встречается так называемый array-driven development — структуры данных представлены массивами. Например, есть легаси-сервис, который регистрирует пользователей. Он принимает массив $params.
final class UserService
{
public function register(array $params): void
{
// ...
}
}
$this->registration->register($params);Код неочевиден. Нам придется залезть в сервис, чтобы узнать, какие поля там используются. Статическим анализом мы можем явно указать, что в этот метод нужен такой-то массив с такими-то ключами, а под ключами — значения определенных типов.
final class UserService
{
/**
* @psalm-param array{name:string, surname:string, email:string, age:integer} $params
*/
public function register(array $params): void
{
// ...
}
}
$this->registration->register($params);Прописав через докблоки формат и тип данных, при следующем рефакторинге мы запустим Psalm, он проверит все возможные вызовы и заранее скажет, сломает ли код. Достаточно удобно при работе с легаси.
Что дальше?
Получите контроль над существующей кодовой базой, поймите, как она работает, и попытайтесь немного привести ее в порядок. Имея в арсенале тесты, статанализ и постепенно вычищая код, вы сможете приступать к более серьезному рефакторингу и улучшать качество продукта.
А дальше — просто подумайте, что еще нужно, чтобы чувствовать себя счастливым, работая с этим кодом. Чего еще не хватает? Составьте себе план действий и постепенно улучшайте доставшийся вам в наследство код.
p.s. Несколько полезных материалов для дальнейшего изучения:
Пост основан на докладе с краснодарского PHP-митапа — вот его запись
