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

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

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

выбрал реакт


Видео


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


  • Плохое 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 и все)

69m32s — e.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

© Habrahabr.ru