Небольшое код ревью, тесты и рефакторинг в Laravel. Плохой/хороший коД

Сегодня будем смотреть код реального проекта, где я работаю в команде с другими разработчиками. Сделаем код ревью и рефакторинг. Буду делиться с Вами процессом работы

#рефакторинг#laravel#cutcode
---------------------------------------------------------------------------------

---------------------------------------------------------------------------------
⏰ Таймкоды:
00:00 Введение
00:43 Обзор проекта
04:10 Создание тестов
05:53 Рефакторинг

Всех приветствую на канале Cutcode! Сегодня стартует новая рубрика, которую я назвал хороший плохой код. Будем смотреть на код, который имеет вопросы, серьезные вопросы, либо с небольшим запашком. И рассматривать детально проблемы, а потом буду демонстрировать код после рефакторинга. В общем и плохой и хороший код нас ждет сегодня. Вы обязательно напишите имеет ли это рубрика право на существование, либо предложите свое решение, если увидеть еще проблемы. Все это приветствуется, обязательно с вами обсудим. Но меньше слов друзья - погнали!

На обзоре у нас сегодня реальный проект, работаем в команде и в процессе рефакторинга и code review сразу делюсь с вами. Думаю это заслуживает хотя бы лайка.

Итак у нас класс контроллер, который отвечает за импорт данных из crm. Данные у нас приходят в json формате каждую ночь. Да возможно в целом подход не из лучших, но мы иногда привязаны к обстоятельствам и особенностям ТЗ. Импорт реализовал один из разработчиков моей команды и скажем так у нас живое code review по проблемам которые здесь есть. Сразу скажу что код рабочий импорт работает без ошибок. Но все-таки что же с ним не так? Во-первых что самое странное, это то, что код писался без тестов. Хотя на мой взгляд и я бы пошел именно таким путем, я бы использовал TDD паттерн и начал именно с тестов. Почему? Ну смотрите разработчик явно действовал следующим образом: писал код, а проверял пуля тестовый запрос либо прямо из crm, либо эмулировал через скажем postman в итоге каждый раз при любом изменении отправлял запрос и ждал ошибку, а в случае если ее нет, то смотрел все ли хорошо в базе, все ли там создалось и именно так как нужно. Лично я слишком ленив для такого подхода и определенно сразу бы написал тесты и далее контролировал бы поведение за счет тестов. Видел бы что запрос прошел валидацию, что все записи создались и они имеют правильные значения. Здесь же помимо того что каждый раз нужно самостоятельно все смотреть, но даже при таком подходе ошибиться и не углядеть что-то крайне легко. Это во-первых и это крайне критично. Во-вторых мне не нравится что здесь нет валидации данных и что мы здесь сразу делаем декодирование, хотя можно было бы перенести этот процесс в отдельный класс как раз валидации. В-третьих у нас идет метод апдейт либо create по полю ID. ID у нас явно уникальное поле. Дублей здесь быть не может и мы получается что на каждый город сразу отправляем к базе два запроса на поиск записи и на обновление либо добавление.

---------------------------------------------------------------------------------
📹 делитесь этим видео с друзьями:

📼 Курс по Laravel с нуля:

Небольшое код ревью, тесты и рефакторинг в Laravel. Плохой/хороший коД

---------------------------------------------------------------------------------

[alexredcross]

Приятно смотреть такие рубрики, а ещё когда тесты адекватно работают, мало кто пишет код и ещё показывает его покрытым тестами, спасибо Данил :)

[timidosUKR]

4:20 Я не знаком с принципами написания тестов в Laravel. Но всё же хочу отметить, что несколько логически отдельных тестов следует оформлять в отдельных блоках (методах). В вашем случае, я вижу 3 теста: "пустой запрос", "запрос с пустым полем `cities`", "правильный запрос".

[aplokhy]

Отличная рубрика, в сети не хватает примеров именно с реальными проектами, а так твой канал выгодно отличается от других, продолжай)

[wotanweb]

Идеальный формат!
Хотелось бы больше таких видео

[Garkolym]

Очень хороший формат, пожалуйста, продолжай.👍

[progtime]

Все понятно! Интересно было увидеть видео по тестирование или серию видео включающие unit, интеграционные и браузенрные тесты

[rosamarsky]

1. TDD - не паттерн, а подход к разработке.
2. Сам то в FormRequest'ах возвращаемые типы не указываешь)
3. Создавать коллекцию, чтобы вызвать map(), а потом снова перевести в массив... нууу.. array_map() для слабаков?:)
4. public function handle($cities) { ... } - тайпхинт ```array $cities```
5. function (array $city) { ... } - да-да, в замыканиях тоже указываем тип.
6. Возвращаемый тип в контроллерах: у тебя указаны юзейджи, необязательно писать полный namespace.

[TsAex]

6:22 Нет возвращаемого типа в rules(), бесполезные php docs можно смело удалять. 6:33 В контроллере можно использовать $request->validated() вместо магического геттера, полные FQCN можно и нужно сокращать (линия справа в редакторе не просто так проведена)
6:39 Что будет, если во входящих данных не будет нужного ключа в массиве? Рекомендую использовать Arr::get() для получения данных в массиве. Я так понимаю при любой ошибке импорт свалится. UpdateOrCreate может всё же лучше будет, так если объем $data будет большой, то будет провал по памяти, а так шанс что хоть часть данных зайдёт. Хотя тут уже от задачи зависит. Предпочтительно при большом объеме импорта работать с чанками
7:32 Куда пропали поля type_id и region_id после рефакторинга? Они не нужны в БД?

Удачного кодинга

[nftdhhy]

Спасибо. интересно! Побольше бы видео с код ревью. А еще лучше на что обращаться внимание при код ревью)

[nikitasomusev]

Отлично, спасибо за контент.
Хотелось бы увидеть разные виды тестирований. На скок я понимаю, тут feature tests

[Naikshy]

Думал тоже за тесты сказать но вижу уже всё прояснили в комментариях, отлично)
Очень удачная длинна ролика, не утомляет смотреть, спасибо

[socialreklamaru]

Формат отличный, продолжай!
Напиши тест, где в cities передаётся строка “test”, кое-что нужно дописать)

[zosyanax]

Топовая рубрика, однозначно продолжать!

[KibokoKwembamba]

Шикарная рубрика!!! Оставляем всё как есть

[ewujnup]

где тайпхин переменной у handle?
замыкание в map можно указать статичным, оно не зависит от текущего окружения, можно указать его тайпхинты

[rzufypb]

Отличный формат!!!! Для новичка просто находка !!!!

[keucmgg]

Спасибо. Рубрика отличная. Продолжайте пожалуйста))

[atoomotr]

Да, это интересно. НО, если будете писать код вместо просто показа, то будет понятнее уследить для новичков.
Спасибо за материал!

[konstantinchernishov]

Рефакторинг огромного файла с роутами в студию пожалуйста=)

[velman]

супер! сделай подробный ролик о написании тестов.