Govnokodu.Net

Неудачные названия методов и способ хранения состояния

Исходный код

loadingDatePopup() {
    this.breakDataPopup = 1;
    setTimeout(() =>; {
      if (this.isSeoPage && !this.dataPopup && this.offsetTitleTop && this.breakDataPopup === 1) {
        this.dataPopup = newToursDatePopup(this.params);
      }
    }, 10000);
  }
eventScrollHandler() {
    if (this.offsetMenuTop && this.breakDataPopup === 1) this.breakDataPopup = 2;

    if (this.offsetTitleTop && this.breakDataPopup !== 2) {
      this.loadingDatePopup();
    }
  }
getoffsetTitleTop() {
    return document.querySelector(this.toursTitle).getBoundingClientRect().top < 0;
  }

  getoffsetMenuTop() {
    return (
      document.querySelector(this.toursRForm).getBoundingClientRect().top +
        document.querySelector(this.toursRForm).getBoundingClientRect().height >  0
    );
  }

Что не так в этом коде

  • запутанная логика показа или непоказа попапа: просто глядя на код, трудно понять, при каких условиях в итоге попап будет показан, а при каких нет (главным образом запутанность возникает из-за многочисленности условий и проверок)
  • неудачные названия геттеров offsetTitleTop и offsetMenuTop — неясно, что конкретно происходит на странице, когда эти геттеры возвращают истину, и что происходит, когда они возвращают ложь
  • неудачное название метода: оно не сформулировано как команда

Как можно отрефакторить код

Для начала переименовать геттеры, чтобы они отражали смысл происходящего на странице:

get isBelowTitle() {
    const titleRect = document.querySelector(this.toursTitle).getBoundingClientRect();
    return titleRect.top < 0;
  }

  get isBelowSearchForm() {
    const formRect = document.querySelector(this.toursRForm).getBoundingClientRect();
    return formRect.top > - formRect.height;
  }

Дальше следует добавить геттер, который нам будет однозначно сообщать, следует ли показывать попап:

get shouldShowPopup() {
    return this.isBelowSearchForm();
  }

Предполагаю, что одного этого условия (поисковая форма уже не видна на странице) вполне достаточно.

Теперь нужно определить методы, которые будут управлять планированием и отменой показа попапа:

scheduleShowDataPopup() {
    this.timerDataPopup = this.timerDataPopup || setTimeout(
      () => this.dataPopup = newToursDatePopup(this.params),
      10000
    );
  }

  cancelShowDataPopup() {
    clearTimeout(this.timerDataPopup);
    this.timerDataPopup = null;
  }

Как видишь, метод планирования теперь знает только о том, как и когда планировать показ попапа, и ничего не знает, при каких условиях это следует делать.

И наконец, сводим всё воедино:

onScroll_dataPopup(event) {
    if (this.shouldShowDataPopup) {
      if (!this.dataPopup) {
        this.scheduleShowDataPopup();
      }
    }
    else {
      this.cancelShowDataPopup();
    }
  }

— в итоге у нас есть обработчик события скролла, который знает об условиях для планирования или отмены показа попапа.

Кстати, при показе попапа можно было бы отписываться от события прокрутки для снижения нагрузки на процессор.

Сергей Галанин