Сделал тестовое задание, ответили что я джун плюс, объясните что я сделал не так

Недавно попросили сделать вот это тестовое задание https://docs.google.com/docume... sp=sharing, сделал вовремя, отправил, вроде всё корректно работает, может кроме тестов, но ответ меня очень удивил, дословно:

«По коду видно что не было опыта с задачами мидл уровня. Реализация на уровне студенческой работы. На мидл уровень мы не можем взять такого кандидата, ему можно начинать с Jun+. Поэтому отказ пока что, нужен сейчас разработчик чуток сильнее».

Ссылка на код https://bitbucket.org/giperilia/smartsatu-test/

Кто- нибудь может объяснить в чем может быть проблема?

0

Контроллеры:
1) Конструкторы дублируются. Если уж делаете какую-то общую логику то можно сделать базовый контроллер, trait, middleware...
2) Хз зачем реквест с респонсом запихали в поля класса.
3) Валидации нет.
4) Всегда отправляете http code = 200. Надо использовать хотя бы основные: 401, 403, 404, 500, 503... Я раньше не понимал зачем, но когда начинаешь настраивать логи и мониторинги, то эти статусы очень помогают.
5) Поля из реквеста берете через магические свойства, тут могут быть конфликты с уже существующими полями в реквесте + IDE наверняка ругается что не находит такие поля.
6) В каждом методе у вас произвольный формат response, это конечно можно держать в голове, но завтра придет человек который этого не знает и сделает по своему, а через неделю вы и сами забудете. Я обычно использую такую библиотеку.

Сервис Order:
1) Логика совершенно не читаемая. Даже зная как оно должно быть по ТЗ, в коде этого не видно.
2) Поле $order почему-то публичное. Т.е. в любой момент можно записать туда null и сломать программу.

БД:
1) Статусы заказа зачем-то в БД зашили. В задании же не сказано что нужна возможность редактировать статусы через админку. Вы просто сделали лишнюю работу, усложнили код, увеличили нагрузку на БД...
2) Транзакции не использовали. Какой-нибудь косяк в БД и целостность данных нарушена.
3) Cart какая-то сомнительная сущность. Всякие там total_price надо динамически считать. Это быстрей, надежней и место в БД не занимает.
4) Удалять что либо не рекомендуется, даже через 'set null', а cascade еще опасней. Лучше использовать мягкое удаление.

Маршруты:
1) Зачем прописывать полный путь к контроллерам?
2) Почитайте чем отличаются post, put, patch и в каких ситуациях их использовать.

Тесты:
1) Какой еще clearCache? Все подобное настраивается через phpunit.xml
2) Проверка на не тестинг это зачем? Неужели вы тесты запускаете в каком-то еще окружении, кроме тестового? Хотите прод сломать?
3) Почему бы не использовать http запросы, зачем вручную создавать контроллеры итп? Роуты и мидлвейры тестами не покрыты получаются.
4) Тестите removeFromCart, а вызываете addToCart. Копипаст это зло)
5) Вызывать тест из другого теста не очень хорошая практика. Так их сложней поддерживать. В данном случае можно было сделать Cart через фабрику.
6) Тесты с БД довольно медленные и их сложно поддерживать. Почитайте про моки.
7) Тесты без assert это как-то странно. Ощущение что делали чисто для вида.

Прочее:
1) В коде встречаются магические числа, например ->where('status', 1). Пробежался по проекту и так не понял чему соответствует 1. Правильно будет создать Enum и писать ->where('status', CartStatus::New). Еще можно все это в scope вынести, но это уже на любителя.
2) Куча всяких IF, да еще и вложенных, сложно читается. Надо рефакторить.
3) Все сделано одним большим коммитом. Не видно ход ваших мыслей. Не понятно где ваш код, а где фреймворка.
4) Консольных команд вообще нету, т.е. 33% задания не сделано. Там наверняка бы сделали ошибку и скопипастили все из контроллеров, вместо того чтобы вынести общую логику в сервисный слой.
А вот вижу какой-то ConsoleController, хз что это, в ларавел это делается по другому.
5) С форматированием кода проблемы. Поставьте себе нормальную IDE и настройте автоформатирование.
6) Тут зачем то передаете $order, хотя сервис его не принимает. На этой же строке получаете реквест через функцию request(), а не из поля класса как в остальных местах.

В общем вам правильно сказали, у вар уровень джун+. Потенциал есть, просто нужен опытный наставник/команда, чтобы вы увидели как сделать лучше. Самому доходить до этого довольно сложно, особенно на тестовых проектах. Видимо у компании нет пары лишних месяцев чтобы дотянуть вас до нужного уровня.

p.s. Рекомендую github.

2

Круто, спасибо за ответ, я процентов 30 из того, что Вы написали сделал бы на существующем проекте, мне сказали что это простое задание на 3-4 часа, то есть им ,по идее, важна была реализация именно со статусами заказа. Я не могу тратить времени на тестовое задание больше, чем день. То есть всякий код вроде это просто заглушка, так как не в этом совсем задача.

С тестами у меня совсем опыта нет, мне было просто интересно их начать хотя бы с этого тестового задания, спасибо, почитаю то , что Вы написали.
Консольные команды работают, они находятся в папке routes, но наверное Вы правы, я должен был их сделать по другому.

А как стоило реализовать статусы заказа? Ну я запихнул их в базу потому что тогда это было бы более масштабируемо.


В целом мне вообще разонравилась концепция тестовых заданий, потому что по идее это должно быть задание, которое выполняется за 2- 3 часа, а в итоге если не сделал кучу всего, что не входило в список требований, то тестовое задание не проходит.

0

Сообщение от captaincacao А как стоило реализовать статусы заказа? Ну я запихнул их в базу потому что тогда это было бы более масштабируемо.

Просто в коде зашить Enum и логику смену статусов. Причем я бы для каждого варианта смена статуса сделал бы свой метод, а не вот эти +-1. Если статус нельзя менять, то бросал бы exception.
Хз как вы представляете масштабирование. Даже простое добавление промежуточного статуса уже смещает все остальные статусы. А более сложная логика туда вообще не залезет, например параллельная ветвь.

Сообщение от captaincacao важна была реализация именно со статусами заказа

Если так рассуждать, то можно было бы просто алгоритм своими словами рассказать за 5 минут)

Сообщение от captaincacao потому что по идее это должно быть задание, которое выполняется за 2- 3 часа

Это несерьезно. Мне сложно представить задачу, в которой хороший специалист смог бы показать свои возможности за это время. Это джуну можно какую-нибудь сортивку дать и всё)
Если в себе уверены, то можно договариваться о каких-то других проверках. Рекомендации с предыдущей работы, готовый код своих проектов, который сможете защитить, просто словами рассказать как бы вы решали такую задачу.

Сообщение от captaincacao а в итоге если не сделал кучу всего, что не входило в список требований, то тестовое задание не проходит.

В том то и прикол, эту джуну надо все разжевывать. Чем опытней специалист, тем меньше его нужно контролировать. Я вот более чем уверен, что на реальном проекте вы также будете халтурить, ибо это привычка.

0