Изучаю ООП JavaScript. Метод в методе?

Question

[SongaerDev]

Всем привет. Потихоньку начинаю вникать в ООП JavaScript (в самом JS тоже относительно недавно), решил написать свой слайдер. Сам код:

window.addEventListener('DOMContentLoaded', () => {
  //slider
  let portfolioSlider = document.querySelector('.portfolio-slider__wrap'); //главный контейнер слайдера
  //Если слайдер есть на странице
  if (portfolioSlider) {
    class PortfolioSlider {
      constructor (autoSlide = 'auto') {
        let sliderItem    = portfolioSlider.querySelectorAll('.slider__item'); // получаем одиночные слайды
        this.numberSlides = sliderItem.length;
        this.autoSlide    = autoSlide;
      }

      //генерируем точки навигации
      createNavDots () {
        if (this.numberSlides !== 1) {
          let sliderDots = portfolioSlider.querySelector('.slider__dots'); //контейнер с точками навигации

          for (let i = 0; i < this.numberSlides; i++) {
            let singleDots = document.createElement('span');

            singleDots.classList.add('single-dot');
            if (i === 0) {
              singleDots.classList.add('active');
            }
            sliderDots.insertAdjacentElement('beforeend', singleDots);
          }
        }
      }

      //управление слайдами
      slidesNavigation () {
        if (this.numberSlides !== 1) {
          let sliderDots    = portfolioSlider.querySelectorAll('.single-dot'),
              sliderItems   = portfolioSlider.querySelector('.slider__items'),
              indexItemPrev = 0,
              indexItem     = 0,
              nextSlide     = portfolioSlider.querySelector('.next-slide'),
              prevSlide     = portfolioSlider.querySelector('.prev-slide'),
              slidePosition = 0;

          if (this.autoSlide === 'auto') {
            setInterval(() => {
              indexItem += 1;
              slidePosition += 100;

              if (indexItem > this.numberSlides-1 || slidePosition > (this.numberSlides-1) * 100) {
                indexItem = 0;
                slidePosition = 0;
              }

              sliderItems.style.transform = 'translateX(-'+ slidePosition +'%)';

              sliderDots[indexItemPrev].classList.remove('active');
              indexItemPrev = indexItem;
              sliderDots[indexItem].classList.add('active');
            }, 2000);
          }

          //отображаем полосы навигации
          nextSlide.style.display = 'block';
          prevSlide.style.display = 'block';

          //обрабатываем клик по точкам навигации
          sliderDots.forEach(function (item) {
            item.addEventListener('click', () => {
              let parentItem = item.parentNode;

              indexItem = Array.prototype.indexOf.call(parentItem.children, item);

              slidePosition = indexItem * 100;
              sliderItems.style.transform = 'translateX(-'+ slidePosition +'%)';

              sliderDots[indexItemPrev].classList.remove('active');
              indexItemPrev = indexItem;
              sliderDots[indexItem].classList.add('active');
            });
          });

          //обрабатываем клик по стрелкам вперед/назад
          prevSlide.addEventListener('click', () => {
            indexItem -= 1;
            slidePosition -= 100;

            if (indexItem < 0 || slidePosition < 0) {
              indexItem = this.numberSlides-1;
              slidePosition = (this.numberSlides-1) * 100;
            }
            console.log(slidePosition);

            sliderItems.style.transform = 'translateX(-'+ slidePosition +'%)';

            sliderDots[indexItemPrev].classList.remove('active');
            indexItemPrev = indexItem;
            sliderDots[indexItem].classList.add('active');
          });

          nextSlide.addEventListener('click', () => {
            indexItem += 1;
            slidePosition += 100;

            if (indexItem > this.numberSlides-1 || slidePosition > (this.numberSlides-1) * 100) {
              indexItem = 0;
              slidePosition = 0;
            }

            sliderItems.style.transform = 'translateX(-'+ slidePosition +'%)';

            sliderDots[indexItemPrev].classList.remove('active');
            indexItemPrev = indexItem;
            sliderDots[indexItem].classList.add('active');
          });
        }
      }
    }
    //создаем экземпляр слайдера
    const portfolioMainSlider = new PortfolioSlider('auto');
    portfolioMainSlider.createNavDots();
    portfolioMainSlider.slidesNavigation();
  }
});

Строго не судите, в JS совсем недавно, реализовал как мог (но, конечно, от конструктивной критики не откажусь) :) Вопрос в следующем. Есть метод slidesNavigation, в котором отслеживает клику по кнопкам навигации и в зависимости от индекса переключаются слайды. В каждом обработчике клика есть кусок кода вида:

sliderDots[indexItemPrev].classList.remove('active');
indexItemPrev = indexItem;
sliderDots[indexItem].classList.add('active');

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

Помогите, пожалуйста, разобраться в этом вопросе. Заранее спасибо!

Comments

[Konata Izumi]

Где тут ООП?
Обычная процедурщина

[Дмитрий]

Что то мне подсказывает это не весь код

[SongaerDev]

Дмитрий, весь, только что перепроверил

[Дмитрий]

https://www.sonarsource.com/docs/CognitiveComplexi...
Почитайте, и в коде станет более проще разбираться
Смысл такой, переписать толстый метод, сделать кучу маленьких, например метод который будет получать индексы

[Дмитрий]

SongaerDev, да я уже нашёл, просто портянку с телефона не шибко просто осиливать

[SongaerDev]

Дмитрий, да, спасибо! буду обмозговывать

[SongaerDev]

Дмитрий, вот как раз вопрос. Метод, получающий индексы, будет вызываться внутри метода навигации слайдов. Но так сделать нельзя, как я понял.

[Дмитрий]

SongaerDev, все можно.. Просто вам надо больше свойств добавить, чтоб сохранять состояние чего либо

Answer 1

[Viktor]

На самом деле всё довольно просто. Можно пойти двумя путями:
1) Создать функцию внутри метода и вызывать её. Простой и удобный способ. Не захламляется класс и не нужно создавать никаких параметров для метода

slidesNavigation() {
  // твой метод, в котором уже есть код
  ...

  // заменяешь повторяющийся фрагмент на вызов функции
  toggleDotClasses();

  // функция, в которую завёрнут часто повторяющийся фрагмент
  function toggleDotClasses() {
    sliderDots[indexItemPrev].classList.remove('active');
    indexItemPrev = indexItem;
    sliderDots[indexItem].classList.add('active');
  }
}

2) Создать новый метод в классе. Так стоит сделать, если есть как минимум подозрение, что этот твой метод может потребоваться в других методах кроме slidesNavigation

class PortfolioSlider {
  ...

  // новый метод класса. нижнее подчеркивание намекает на то, что метод как бы приватный (служебный)
  _toggleDotClasses(sliderDots, indexItemPrev, indexItem) {
    sliderDots[indexItemPrev].classList.remove('active');
    indexItemPrev = indexItem;
    sliderDots[indexItem].classList.add('active');
  }

  slidesNavigation () {
    this._toggleDotClasses(sliderDots, indexItemPrev, indexItem);
  }
}

На мой взгляд лучше использовать первый вариант. В текущей реализации класса второй вариант не очень удобен

Comments

[SongaerDev]

Да, вроде разобрался, спасибо!

Answer 2

[bini1988]

Как вариант:

toggleActiveDot(activeIndex) {
    Array.from(sliderDots).forEach((dot, index) => {
       if (index === activeIndex) {
          dot.classList.add('active');
      } else {
          dot.classList.remove('active');
      } 
}