mpreterer / Hotel-Toxin

0 stars 0 forks source link

JS #11

Closed metanonum closed 2 years ago

metanonum commented 2 years ago
  1. Лучше все публичные методы держать вверху класса JS 8.5
    Скриншот 22-08-2022 013743
  2. Имена классов без js- префикса нужно брать из констант. В данном случае можно сложить их в объект в button-like/utils/buttonLikeClassNames.js Скриншот 22-08-2022 014205
  3. Ищем только по js-. К body лучше обращаться через document.body, но только там где это действительно нужно.
    Скриншот 22-08-2022 014726
  4. Не нужно использовать анонимные обработчики
    Скриншот 22-08-2022 015101
  5. Нужно избегать вложенных условий. Например здесь одну вложенность можно убрать поправив первое условие (ну и анонимный обработчик тоже нужно убрать) Скриншот 22-08-2022 015142
  6. Все проверки содержащие больше одного условия должны быть внесены JS 1.9
  7. Тоже самое касается магических чисел. (Здесь "8")
  8. Ширина строки должна быть не больше чем 70-80 столбцов. Это можно настроить в prettier. (также касается .pug)
    Скриншот 22-08-2022 015719
  9. Классы с js- префиксами не имеют отношения к БЭМ, поэтому лучше подобрать им подходящие названия в lowercase-hyphenated (можно схожие с теми что есть)
metanonum commented 2 years ago

11.5

Скриншот 31-08-2022 224853 Здесь можно так:

if (this.container.contains(event.target)) return;
this.counterPanel.classList.remove(typeDropDown.BLOCK_OPEN);
this.counterPanel.classList.add(typeDropDown.BLOCK_CLOSE);
if (this.open.classList.contains(typeDropDown.NAME_ACTIVE)) {
  this.open.classList.remove(typeDropDown.NAME_ACTIVE);
}

Есть еще другие места.

11.6

Не все проверки содержащие более одного условия вынесены.
Скриншот 31-08-2022 225847 В данном случае лучше наверное подумать как вообще избавиться от этой цепочки if else.

metanonum commented 2 years ago

11.4

Это тоже анонимный обработчик.
Скриншот 12-09-2022 182210
И еще, в данном случае, если хочешь изменить стили после отображения первой картинки, то лучше на нее и повесить обработчик, а стили лучше менять через соответствующий модификатор.

11.5

Скриншот 12-09-2022 183836

if (date === undefined) return;
...

11.6

Скриншот 12-09-2022 183850 Имена это хорошо, но тут 4 условия. JS 1.9 Скриншот 12-09-2022 205824 Тут по 3 условия, отрицательные утверждения и длинная цепочка if else, что вкупе не читабельно JS 1.1
Почему бы не отрефакторить этот участок, например так:

const options = [strBedRooms, strBed, strBathRoom];
const optionsMaxCount = 2;
const selectedOptions = options.filter((option) => option !== '');
if (selectedOptions.length <= optionsMaxCount) {
  this.inputName.value = selectedOptions.join(', ');
} else {
  this.inputName.value = `${selectedOptions.slice(0, optionsMaxCount).join(', ')}...`;
}

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

metanonum commented 2 years ago
  1. Отрицательные утверждения JS 2.5 Скриншот 12-09-2022 183419