@marinaabcd
Frontend Developer

Над чем нужно работать, что улучшать?

Всем привет,

учу frontend, прохожу курсы Hexlet,
сделала свое первое тестовое задание от Aviasales на React:
https://marinarodkin.github.io/aviasales-app/
https://github.com/marinarodkin/aviasales-app

Как можно оценить мой уровень? Над чем нужно работать, в какую сторону? Буду делать еще несколько тестовых заданий для гит хаб. Цель - пойти на работу front-end'ом,

сама могу сказать, что как минимум, нужно доделать responsive, единицы измерения в CSS перевести в относительные,
логику фильтров упростить (но нужна подсказка как)

спасибо заранее за ответы!
  • Вопрос задан
  • 4455 просмотров
Решения вопроса 1
Vlad_IT
@Vlad_IT
Front-end разработчик
  1. Закомментированный код на гитхабе - не хорошо. https://github.com/marinarodkin/aviasales-app/blob...
  2. Минимум логики в render функции компонента. Все сложные конструкции переносите в методы, а лучше в отдельные компоненты (тогда сможете легче контролировать перерисовку компонентов через shouldComponentUpdate , чтобы они не перерисовывались, если данные не поменялись). Вы можете прямо как методы писать стрелочные функции:
    class Flight extends Component {
        getWeekDay = (date) => {
            //..
        }
        // ....
    }

  3. Вы в половине случаев используете точку с запятой, а в половине нет. Используйте чаще.
  4. Атрибут for нельзя использовать в jsx (как и class, как вы знаете). Вместо for пишите htmlFor
  5. Смотрите консоль инструментов разработчика, там есть ошибки.
  6. Освойте shouldComponentUpdate, он позволяет контролировать перерисовку компонента при изменении состояния или пропсов. У вас при изменении кол-во пересадок, перерисовывается весь список билетов, даже те, которые уже были в этом списке. Многие скажут, что еще рано такое учить, но я не согласен. Если не учиться контролировать перерисовку еще в начале обучения, то можно написать очень много тормознутого софта.
  7. У вас данные ticket.json подгружаются хардкодно из github, это не хорошо, т.к. этот файлик с данными есть в папке public, и если потенциальный работодатель захочет поменять там что-то, он не увидит изменений (т.к. грузится с гитхаба).
  8. У вас если в данных в параметре departure_date стоит 11.10.2018 (т.к. сегодня), то отобразится это как "11 окт 2018, вс", т.е. день недели неправильный. А он неправильный потому, что это не октябрь, а ноябрь. Ошибка в методе getDateFormat
    const newDate  = new Date (year, month, day, );
    const monthName = ["дек", "янв", "фев", "мар", "апр", "мая", "июня", "июля", "авг", "сент", "окт", "ноя", "дек"];
    const newMonth = monthName[newDate.getMonth()];

    конструктор Date вторым аргументом ожидает номер месяца, нумерация которого начинается с нуля. т.е. 0 - январь, 1 - февраль, 11 - декабрь. Судя по monthName вы подозвевали, что есть что-то неладное, но ошибись с реализацией. monthName должен иметь обычный вид, начинаться с января и заканчиваться декабрем, т.к. нулевой элемент массива как раз подходит по логике с нулевым месяцем. В getDateFormat, а также в getWeekDay, вычтите из month - 1
    const newDate = new Date(year, month - 1, day)
  9. У вас в тех же getDateFormat и getWeekDay в конструкторе Date вы в конце аргументов пишите запятую, так не нужно делать. Это не вредно и не полезно, просто дурной тон. Там в любом случае будет undefined, хоть с запятой хоть без нее.
  10. Картинки тоже грузятся с marinarodkin.github.io, измените.

  11. const getStopsNumber = (stop) =>{
          switch (stop) {
            case 3:
              return "3 пересадки"
            case 2:
              return "2 пересадки"
            case 1:
              return "1 пересадка"
            case 0:
              return "без пересадок"
            default:
              return // это не нужно делать, писать return. Если вы удалите эту (и строку выше), то результат будет такой же - undefined
          }
        }

  12. Если бы в SideBar пропс stopsData был не объектом, а строкой или числом, то компонент SideBar можно было бы безболезненно превратить в PureComponent. Ну это так, к слову об оптимизации.
  13. Я бы в stopsClick передавал не объект события e, из которого вы потом берете id элемента через e.target.id (что не есть гуд), а сделал бы стрелочную функцию (или bind), в которую бы передавал id. Вот так
    <input onClick={() => this.props.stopsClick("allStops")} />
    <input onClick={() => this.props.stopsClick("noStops")} />

    Если это читают опытные ReactJS разработчики, рассудите пожалуйста. Согласен, что на каждый компонент будет создана своя копия функции, но по крайней мере, не нужно взаимодействовать с DOM напрямую.
  14. Это не красиво
    if( this.state.stops.allStops === false && this.state.stops.noStops === true && this.state.stops.oneStop === true && this.state.stops.twoStop === true && this.state.stops.threeStop === true  ){
             newStops = {...this.state.stops, allStops: true}
        }

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


Может я многое высосал из пальца, но это будет вам полезно. Учитесь, развивайтесь. Удачи вам в этом :-)
Ответ написан
Пригласить эксперта
Ответы на вопрос 5
sfi0zy
@sfi0zy
Creative frontend developer
Не любитель реакта, поэтому про него не буду говорить. А вот CSS покритикую:
- Стоит прикрутить какой-нибудь препроцессор, поиспользовать вложенность (структура лучше будет видна) и вынести в человеко-понятные константы все, что выносится - цвета, размеры и.т.д. Там достаточно повторений.
- Стоит поделить все на отдельные файлы-компоненты.
- Стоит получше подумать над общим разбиением CSS на компоненты. Есть конечно разные подходы, но отдельные кнопки, или группа из нескольких кнопок, или чекбоксы - это универсальные штуки на весь проект. Какой смысл их привязывать к какому-то сайдбару или калькулятору?
- Про адаптивность вы сами написали.
- Стили для :focus отсутствуют. Клавиатурой не получится пользоваться.
- Еще мне кажется, что у сайдбара отступ внизу должен быть (дизайн не видел, но имхо есть). И что cursor: pointer у кнопок должен быть.

З.Ы.: Еще есть мысль, что вариант "все" там не нужен. "Все" должны показываться при отсутствии фильтров. Но без анализа ЦА не буду утверждать, может там к к такому варианту люди привыкли.
Ответ написан
@spaceatmoon
Ну вот чисто взгляд со стороны.

1. Научитесь оформлять проекты через markdown. Сейчас такое описание проекта тяжело читать, я осилил 5 слов, потенциальный работодатель вообще не посмотрел бы.
2. Группируйте файлы проекта по типу и смыслу. Сейчас это каша и трудно понять мне как простому не фронтендеру где логика, а где фреймфорк.
3. Так как вы пишите все еще говнокод, то приучайтесь над сложными участками кода писать комментарии где выражена суть исполнения функции.
4. Не знаю как во фронтенде сейчас, но бекендеры не любят когда мешают логику и шаблоны.
5. Научитесь раставлять скобки, прочитайте как оформлять код в js
spoiler
componentDidMount() {
    this.getTicketData("ticket.json");
      } // И так везде

6. Это что?
spoiler
newArr = [...newArr, ...arr0, ...arr1, ...arr2, ...arr3];

7. Это плохая практика. Код должен быт абстрагирован от данных. Нужно заменить на объект и проверять есть ли в объекте нужные данные или вернуть дефолтное значение.
spoiler
const getStopsNumber = (stop) =>{
      switch (stop) {
        case 3:
          return "3 пересадки"
        case 2:
          return "2 пересадки"
        case 1:
          return "1 пересадка"
        case 0:
          return "без пересадок"
        default:
          return
      }
    }


Пока хватит.
Ответ написан
DarthJS
@DarthJS
Добавлю. Вы работаете с Реактом, но при этом просто повставляли темплейты, что не имеет смысла, вы там можете взять абсолютно любой фреймворк и вообще без фреймворка закинуть темплейты и понавешивать хендлеры. Темболейт нужно разбивать на компоненты, в которые вы прокидываете данные через props, а так у вас много повторяющегося html
Поставьте линтер, он будет вам помагать. С ним вы будете хоть и говнокодить, но красиво
Ответ написан
mbelskiy
@mbelskiy
Software Developer
Заметил что у вас много функций определяется в методах render. В этом нет необходимости, а скорее нужно избегать - либо выносить на уровень класса, если нужен this, либо выносить за его пределы.
Еще вы ходите за картинкой и json файлом по ссылке на гитхаб, а можно их подключить через import, они ведь локально доступны для реакт приложения. Если с json еще спорно, то картинку в шапке точно нужно import-ить
Ответ написан
HalfBloodPrince
@HalfBloodPrince
Front-End Developer
Это вообще по js коду, чем чисто к реакту.
switch(curr){
      case "eur":
        newCurrency = {...this.state.currencyData, rub: false, usd: false, eur: true };
        break
      case "usd":
        newCurrency = {...this.state.currencyData, rub: false, usd: true, eur: false };
        break
      default:
        newCurrency = {...this.state.currencyData, rub: true, usd: false, eur: false };
        break
      }


А если будет еще валюта, будете еще свойства добавлять? Лучше держать свойство currency и от него отталкиваться. Был бы typescript, еще б указали какие строки туда можно куда присваивать. Причем в state currency уже есть, зачем currencyData я не понял.

Тоже самое со всеми stop свойствами, их много, трудно понять, что к чему. В общем, посмотрите на это в первую очередь.
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы