В этой главе я попытаюсь показать как обычно проекты, написанные в стиле документации к фреймворку, растут и решают возникающие проблемы. Начнём с простого примера:
public function store(Request $request)
{
$this->validate($request, [
'email' => 'required|email',
]);
$user = User::create($request->all());
if(!$user) {
return redirect()->back()->withMessage('...');
}
return redirect()->route('users');
}
public function update($id, Request $request)
{
// все примерно также как и в store()
}Это было просто написать. Позже появились новые требования — добавить загрузку аватара и отправка email пользователю после создания.
public function store(Request $request)
{
$this->validate($request, [
'email' => 'required|email',
'avatar' => 'required|image',
]);
$avatarFileName = ...;
\Storage::disk('s3')->put(
$avatarFileName, $request->file('avatar'));
$user = new User($request->except('avatar'));
$user->avatarUrl = $avatarFileName;
$user->save();
\Email::send($user, 'Hi email');
return redirect()->route('users');
}Какая-то логика должна быть скопирована в update метод, но, например, отправка email должна быть только после создания. Код всё ещё выглядит неплохо, но количество дублированного кода растет. Новое требование — автоматически проверять загружаемые картинки на неподобающий контент. Некоторые разработчики просто добавят этот код в store метод и скопируют его в update метод. Более опытные выделят эту логику в новый метод контроллера и вызовут этот метод в обоих местах. Еще более опытные Laravel-разработчики найдут, что код для загрузки изображения стал довольно большим и создадут отдельный класс, например ImageUploader, который будет содержать логику загрузки и проверки содержания изображений. Также будет создан Laravel-фасад (реализация шаблона Service Locator) \ImageUploader для более простого доступа к этому классу.
class ImageUploader
{
/**
* @returns bool|string
*/
public function upload(UploadedFile $file) {...}
}ImageUploader::upload метод возвращает false если загрузка была неуспешной, например, при ошибке облачного хранилища или неприемлемом контенте, или URL-адрес картинки при удачной загрузке.
public function store(Request $request)
{
...
$avatarFileName = \ImageUploader::upload(
$request->file('avatar')
);
if ($avatarFileName === false) {
return %some_error%;
}
...
}Методы контроллера стали проще, поскольку логика загрузки аватаров вынесена в другой класс. Отлично! Если на проекте возникнет необходимость загружать другие картинки, то нужный класс уже готов к использованию! Необходимо только добавить новый параметр в метод upload — папку, куда сохранять картинки.
public function upload(UploadedFile $file, string $folder)Новое требование — немедленно забанить пользователя, который загрузил неприемлемый контент. Звучит немного странно, учитывая неидеальную точность современных анализаторов изображений, но это было настоящим требованием на одном из моих проектов!
public function upload(UploadedFile $file, string $folder)
{
...
if (check failed) {
$this->banUser(\Auth::user());
}
...
}Новое требование — не банить пользователя, если неприемлемый контент был загружен в приватные места.
public function upload(
UploadedFile $file,
string $folder,
bool $dontBan = false)Когда я говорю «новое требование» это не означает, что оно появляется на следующий день. В больших проектах между этими «новыми требованиями» могут пройти месяцы или годы. Их реализацией могут заниматься другие разработчики, которые не понимают почему этот код был написан таким образом. Его задача — просто реализовать это требование в коде, по возможности сделав это быстро. Даже если ему не нравится какая-то часть кода, ему трудно оценить время на рефакторинг. А также, что более важно, трудно не сломать что-либо. Это довольно частая проблема. Я надеюсь, эта книга поможет организовать ваш код так, чтобы его было легко рефакторить.
Новое требование — приватные места пользователя должны иметь менее строгие правила проверки контента.
public function upload(
UploadedFile $file,
string $folder,
bool $dontBan = false,
bool $weakerRules = false)Последнее требование для этого примера — приложение не должно банить сразу. Только после нескольких попыток загрузить неприемлемый контент.
public function upload(
UploadedFile $file,
string $folder,
bool $dontBan = false,
bool $weakerRules = false,
int $banThreshold = 5)
{
//...
if (check failed && !$dontBan) {
if (\RateLimiter::tooManyAttempts(..., $banThreshold)) {
$this->banUser(\Auth::user());
}
}
//...
}Теперь код уже не выглядит хорошо. Функция загрузки изображения имеет кучу странных параметров про проверку контента и бан юзеров. Если процесс бана юзера изменится, разработчик должен открыть класс ImageUploader и реализовывать изменения там. Вызов метода upload выглядит плохо:
\ImageUploader::upload(
$request->file('avatar'), 'avatars', true, false
);Здесь был нарушен Принцип единственной ответственности (Single Responsibility Principle). Класс ImageUploader имеет много других проблем, но мы поговорим о них позже. Как я говорил раньше, методы store и update почти одинаковы. Представим большую сущность с огромной логикой, такой как загрузка картинок, вызов других API и т.д.:
public function store(Request $request)
{
// тонны кода, особенно если
// некая общая логика не была
// вынесена в классы как ImageUploader
}
public function update($id, Request $request)
{
// почти такие же тонны кода
}Иногда разработчик пытается избавиться от дублирования кода выносом всей общей логики в такой метод:
protected function updateOrCreateSomething(..., boolean $update)
{
if ($update)...
if ($update)...
if (!$update)...
}Я видел подобный метод длиной более 700 строк. После многих изменений требований в нем было огромное число if ($update) проверок. Это определенно неправильный вариант исправления дублирования кода. Когда я отрефакторил этот метод созданием разных методов create and update и выносом общений логики в нужные методы и классы, код стал в разы проще для чтения.
Подход RESTful очень популярен. Laravel-разработчики используют ресурсные контроллеры с готовыми методами store, update, delete, и т.д. даже для web роутов, не только для API. Он выглядит очень просто. Всего 4 глагола: GET (прочитать), POST (создать), PUT/PATCH (изменить) и DELETE (удалить). Он очень прост, когда проект — это просто CRUD (Create, Read, Update, Delete) приложение с формами для создания/редактирования и списками сущностей с кнопкой «Удалить». Но когда приложение становится более сложным, подход RESTful становится намного сложнее. Например, я погуглил фразу «REST API ban user» и первые три результата с примерами из документаций к разным API были очень разными.
PUT /api/users/banstatus
params:
UserID
IsBanned
Description
POST /api/users/ban userId reason
POST /api/users/un-ban userId
PUT /api/users/{id}/status
params:
status: guest, regular, banned, quarantine
Там также была огромная таблица: какие статусы могут быть
изменены на какие и что при этом произойдет
Как видите, любой нестандартный глагол — и RESTful подход становится весьма неоднозначным, особенно для начинающих. Обычно все методы реализовываются через метод изменения сущности. Когда я спросил на одном из своих семинаров «Как бы вы реализовали бан юзера в своем REST API?», первый ответ был:
PUT /api/users/{id}
params:
IsBanned=true
Хорошо. IsBanned это свойство сущности User, но когда пользователь реально будет забанен, то приложение должно, например, послать письмо этому пользователю. Чтобы реализовать это требование должны быть реализованы не самые простые проверки «старых» и «новых» значений.
Другой пример: изменение пароля.
PUT /api/users/{id}
params:
oldPassword=***
password=***
oldPassword не является свойством сущности User. Таким образом, еще одно условие в методе update, который становится все сложнее и сложнее. Я все время вспоминаю картинку «типичный продукт Apple, типичный продукт Google» (погуглите как-нибудь) как лучшую иллюстрацию проблемы.
Иногда разработчики не видят (или не хотят видеть) простого пути реализации чего-либо. Они пишут код с рефлексией, магическими методами или другими динамическими фичами языка PHP. Код, который было трудно писать и будет намного труднее читать. Я постоянно этим занимался. Как каждый разработчик, я думаю.
Я покажу один веселый пример. Я написал простой класс для работы с ключами кэширования для одного из проектов. Ключи кэширования нужны как минимум в двух местах: при чтении из кэша и при удалении значений оттуда до срока. Очевидное решение:
final class CacheKeys
{
public static function getUserByIdKey(int $id)
{
return sprintf('user_%d_%d', $id, User::VERSION);
}
public static function getUserByEmailKey(string $email)
{
return sprintf('user_email_%s_%d',
$email,
User::VERSION);
}
//...
}
$key = CacheKeys::getUserByIdKey($id);Помните догму «Не используйте статические функции!». Почти всегда она верна, но это хороший пример исключения. Мы поговорим об этом в главе про внедрение зависимостей. Когда в другом проекте возникла такая же необходимость я показал этот класс разработчику сказав, что можно сделать также. Чуть погодя он сказал, что этот класс «не очень красивый» и показал свой вариант:
/**
* @method static string getUserByIdKey(int $id)
* @method static string getUserByEmailKey(string $email)
*/
class CacheKeys
{
const USER_BY_ID = 'user_%d';
const USER_BY_EMAIL = 'user_email_%s';
public static function __callStatic(
string $name, array $arguments)
{
$cacheString = static::getCacheKeyString($name);
return call_user_func_array('sprintf',
array_prepend($arguments, $cacheString));
}
protected static function getCacheKeyString(string $input)
{
return constant('static::' . static::getConstName($input));
}
protected static function getConstName(string $input)
{
return strtoupper(
static::fromCamelCase(
substr($input, 3, strlen($input) - 6))
);
}
protected static function fromCamelCase(string $input)
{
preg_match_all('<huge regexp>', $input, $matches);
$ret = $matches[0];
foreach ($ret as &$match) {
$match = $match == strtoupper($match)
? strtolower($match)
: lcfirst($match);
}
return implode('_', $ret);
}
}
$key = CacheKeys::getUserById($id);Этот код трансформирует строки вида «getUserById» в строки «USER_BY_ID» и использует значение константы с таким именем. Большое количество разработчиков, особенно те, кто помоложе, обожают писать подобный «красивый» код. Иногда этот код позволяет сэкономить несколько строк кода. Иногда нет. Но он всегда будет крайне сложным в отладке и поддержке. Разработчик должен подумать раз 10 прежде, чем использовать подобные «крутые» возможности языка.
Некоторые разработчики фреймворков тоже любят динамические возможности и тоже реализуют подобную «магию».
Она помогает быстро реализовывать простые мелкие проекты, но используя подобную магию разработчик теряет контроль над выполнением кода приложения и когда проект растет, это превращается в проблему.
В прошлом примере было упущено использование констант *::VERSION, поскольку используя такую динамику, трудно как-либо изменить логику.
Другой пример: Laravel приложения часто содержат много подобного кода:
class UserController
{
public function update($id)
{
$user = User::find($id);
if ($user === null) {
abort(404);
}
//...
}
}Laravel, начиная с какой-то версии предлагает использовать «implicit route binding». Этот код работает также, как и предыдущий:
Route::post('api/users/{user}', 'UserController@update');
class UserController
{
public function update(User $user)
{
//...
}
}Это действительно выглядит лучше и позволяет избавиться от некоторого количества дублированного кода. Когда проект немного вырастет, разработчики начнут внедрять кеширование. Проблема в том, что кеширование можно применять для запросов чтения (GET), но не для запросов записи (POST). Подробнее об этом в главе про CQRS. Разделение операций чтения и записи станет намного больше, если проект начнет использовать разные базы данных для чтения и записи (это случается довольно часто в проектах с высокой нагрузкой). Laravel позволяет довольно легко сконфигурировать подобную работу с базами для чтения и записи. Код с использованием «implicit route binding» будет выглядеть вот так:
Route::bind('user', function ($id) {
// получить и вернуть закешированного юзера или abort(404);
});
Route::bind('userToWrite', function ($id) {
return App\User::onWriteConnection()->find($id)() ?? abort(404);
});
Route::get('api/users/{user}', 'UserController@edit');
Route::post('api/users/{userToWrite}', 'UserController@update');Этот код выглядит очень странно и легко позволяет сделать ошибку. Это произошло потому, что вместо явного запроса сущности по id разработчики использовали неявную «оптимизацию» и потеряли контроль над своим кодом. Пример мог быть сокращен подобным образом:
class UserController
{
public function update($id)
{
$user = User::findOrFail($id);
//...
}
}Нет смысла «оптимизировать» эту одну строчку кода. Фреймворки предлагают много возможностей потерять контроль над своим кодом. Нужно быть весьма осторожными с ними.
Как небольшой итог могу сказать следующее: чем меньше «магии» в коде, тем легче его читать и поддерживать». В очень редких случаях, таких как ORM, нормально использовать магию, но только там и даже тогда не стоит сильно увлекаться.
Когда я учился программированию в университете, мы любили показывать другим примеры суперкороткого кода. Обычно это была одна строка кода, реализующая какой-то алгоритм. Если автор этой строки пытался понять её спустя несколько дней — это занимало несколько минут, но это все равно был «крутой» код.
Мои характеристики хорошего кода сильно изменились с тех пор. Хороший код для меня — это код, который требует минимальное время для своего понимания. Самый короткий код далеко не всегда самый читабельный. Обычно, несколько простых классов намного лучше, чем один, но сложный класс.
Еще один веселый пример с одного из моих проектов:
public function userBlockage(
UserBlockageRequest $request, $userId)
{
/** @var User $user */
$user = User::findOrFail($userId);
$done = $user->getIsBlocked()
? $user->unblock()
: $user->block($request->get('reason'));
return response()->json([
'status' => $done,
'blocked' => $user->getIsBlocked()
]);
}Разработчик хотел сэкономить пару строк кода и реализовал блокировку и разблокировку пользователя одним методом. Проблемы начались с наименования этого метода. Неудачное существительное blockage вместо естественных глаголов block и unblock. Основная проблема этого кода — конкурентность. Когда два модератора одновременно захотят заблокировать одного пользователя, первый его заблокирует, а второй — разблокирует. Оптимистичная блокировка помогла бы с этой проблемой, но она очень непопулярна в Laravel проектах (я нашел лишь пару пакетов для ее реализации, но все они имеют меньше 50 звезд на Github). Лучшее решение — отдельные методы для блокировки и разблокировки.
Я забыл рассказать о главном враге — Copy-Paste Driven Development. Я надеюсь, что ответ на вопрос «Почему дублированный код сложен в поддержке?» очевиден. Существует много способов разработчикам усложнить себе жизнь на проекте. Эта книга о том, как обойти их и про хорошие привычки, позволяющие создавать легко расширяющийся и поддерживаемый код. Давайте начнем!