Pull to refresh

Code-review тестового задания junior react разработчиков

Reading time 3 min
Views 19K

Что это такое?


Это код-ревью решений второго тестового задания. На видео отмечены удачные решения и указаны ошибки, а так же советы по их исправлению. В данной заметке отмечены общие проблемы и даны ссылки с "отметкой времени".


выбрал реакт


Видео

Общие проблемы


  • Плохое Readme;
  • Остаются eslint предупреждения, лишние console.log (redux-логгер не в счет);
  • Иконка Web не вынесен вперед (невнимательное чтение задачи);
  • Иконка Web выносится вперед в компоненте (а лучше бы в редьюсере, или экшене);
  • Пароль не очищается, если запрос вернулся с ошибкой;
  • Submit кнопка в форме логина доступна, если поля пустые (или одно из полей);
  • Submit кнопка в форме логина не поддерживает нажатие Enter;
  • Нет деления на компоненты/контейнеры (не относится к тем, кто делил по другим подходам);
  • URL-адрес для запросов на сервер полностью передается (нет оформления повторяющейся части строки в константу);
  • Ошибка/уведомление "неправильное имя пользователя/пароль" — не очищается;
  • Ошибка "неправильное имя пользователя/пароль" — выводится константой с сервера;
  • Текст ошибки "захардкожен" в коде. Нет обращения в словарик по константе с сервера;
  • Не удален "старый код", то есть такой код, который нигде не используется;
  • Нет блока catch у промисов, нет обработки ошибок, если сервер отвечает не ok;
  • Компоненты размещены в node_modules;
  • Отсутствуют или недостаточно подробно описаны Prop Types.
  • Экшены и редьюсеры в куче в одном файле (или в одном все экшены, в другом все редьюсеры). Нет деления на "модули", то есть каждой сущности — свои экшены и свои редьюсеры;

Все решения с отметками по времени


Здесь указаны только ошибки, и не отмечены хорошие моменты, которых великое множество, поэтому крайне рекомендую посмотреть все подряд тем, кто считает себя "джуном" в разработке на react.




6m00s — Артур Донковцев Commit


7m40s — опечатка в названии функции


8m07s — асинхронные запросы не вынесены в экшены




9m30s — Павел Пимкин Commit


10m07s — все экшены в одном файле. Нет деления на модули.


10m25s — вынос иконки (перебор данных) сделан в компоненте. Лучше в редьюсере или экшене.




11m42s Сергей ZackFox Commit


12m28s — "прикольные" надписи. Лучше делать "нейтрально", чтобы затем подобные задания можно было сразу посылать работодателю.


13m05s — лишний экшен, указывающий что "загрузка" завершилась. То есть вместо трех экшенов: REQUEST / SUCCESS / STOP, можно уложиться в два: REQUEST / SUCCESS.




16m16s — Дмитрий Петров Commit


18m16s — использование var


18m34s — не вынесена часть URL адреса в константу




21m15s — Ефим Хлебный Commit


21m17s — плохое сообщение в коммите


22m15s — одинаковые названия экшенов.




24m16s — Кацура Владислав Commit


25m17s — (не ошибка) — данные приготовлены в редьюсере


27m38s — использование e.target, лучше e.currentTarget


28m20s==, а надо бы ===


28m33s — использование componentWillUnmount


29m00s (не ошибка) — рассуждение про "до серверную валидацию".


30m05s — код не отфморатирован (на любителя)




30m33s — Максим Сафин Commit


31m35s — использование "не универсального" обработчика, там где это уместно.




32m02s — Сергей Regies Linkas Commit


33m42s — нет экшена для прелоадера


34m30s — порядок методов в компоненте. (eslint плагин)


35m30s — не существующий PropTypes




35m57s — Кононов Виталий Commit




38m02s — Ренат Рысаев Commit


39m45s — не делаем то, что не интересно




40m31s — Евгений Санжиев Commit


41m20s (не ошибка) — словарик для работы с ошибками




42m46s — Виталий Набережный Commit


42m54s — не убраны тестовые данные




44m50s — Вениамин Трепачко Commit


Ачивка: очень классный дизайн.


47m42s — redux версия не полноценная.




47m57s — Ingvarr6 (Игорь) Commit


48m21s — нет 404 роута




51m20s — Екатерина Н Commit


51m30s — не очищается ошибка




54m48s — Роман Палесика Commit


55m30s — не хватает экшенов на загрузку/ошибку


56m49s — использование side-effects в редьюсере


58m10s — (не ошибка) вынос иконки web с помощью css (sick!)




58m53s — Умяр Юсупов Commit


59m15s — использование callback'a в setState, что приводит к лишней перерисовке. Лучше валидировать прямо в рендере.


61m01s — неуместное использование else if




62m13s — dsfcv d (boortcore) Commit




63m15s Константин Липский Commit


65m11s — в экшен передается URL целиком, лучше просто id передавать в данном варианте.




67m14s — Ikaow Ikaow Commit


67m50s — сложное условие в shouldComponentUpdate, можно проще (сразу проверить на props.data и все)


69m32se.preventDefault не первый в обработчике




70m01s — Ali Gasymov Commit




71m50s — Ахметанов Альберт Commit


72m20s — компоненты в node_modules


73m15s — дублирование обращений к переменным




74m04s — Женя Белый Commit


76m04s — privateRoute не вынесен в отдельный компонент


76m33s — сложный код для перемещения иконки web


76m56s — избыточное свойство loaded




77m35s — Аладьин Александр Commit


80m33s — ошибка не вынесена в словарик




81m19s — Misha Mihail Commit


81m43s — избыточное использование withRouter




83m04s — Dmitrii Shapovalenko Commit




84m00s — Даниил Commit


84m58s — избыточное деление экшенов


85m55s — ошибка в названии lifecycle-метода




86m58s — Порошин Роман Commit


87m15s — семантически неверное использование тэга article


90m46s — лишний вызов метода массива




91m10s — Артем Бочков Commit

Tags:
Hubs:
If this publication inspired you and you want to support the author, do not hesitate to click on the button
+10
Comments 25
Comments Comments 25

Articles