🛠 Carousel: Second refactor

Hey ,

I'm thrilled to help you learn JavaScript. Unfortunately, you've landed on a page where you cannot access with your current purchase.

Please upgrade (use this link) access this content.

I'm super eager to help you learn more!

🛠 Carousel: Second refactor

Source

Here’s the code we have for all three event listeners:

nextButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const nextSlide = currentSlide.nextElementSibling
  const nextSlideIndex = slides.findIndex(slide => slide === nextSlide)
  const currentDot = dotsContainer.querySelector('.is-selected')
  const nextDot = currentDot.nextElementSibling

  switchSlide(currentSlide, nextSlide)
  highlightDot(currentDot, nextDot)
  showHideArrowButtons(nextSlideIndex)
})

previousButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const previousSlide = currentSlide.previousElementSibling
  const previousSlideIndex = slides.findIndex(slide => slide === previousSlide)
  const currentDot = dotsContainer.querySelector('.is-selected')
  const previousDot = currentDot.previousElementSibling

  switchSlide(currentSlide, previousSlide)
  highlightDot(currentDot, previousDot)
  showHideArrowButtons(previousSlideIndex)
})

dotsContainer.addEventListener('click', event => {
  const dot = event.target.closest('button')
  if (!dot) return

  const currentSlide = contents.querySelector('.is-selected')
  const currentDot = dotsContainer.querySelector('.is-selected')
  const targetSlideIndex = dots.findIndex(d => d === dot)
  const slideToShow = slides[targetSlideIndex]

  switchSlide(currentSlide, slideToShow)
  highlightDot(currentDot, dot)
  showHideArrowButtons(targetSlideIndex)
})

It’s much better than the code we used to have. They’re easier to read and understand.

But we can still improve them.

If you look closely at nextButton's event listener, you’ll notice we use three types of values:

  1. slides
  2. dots
  3. indexes
nextButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const nextSlide = currentSlide.nextElementSibling
  const nextSlideIndex = slides.findIndex(slide => slide === nextSlide)
  const currentDot = dotsContainer.querySelector('.is-selected')
  const nextDot = currentDot.nextElementSibling

  switchSlide(currentSlide, nextSlide)
  highlightDot(currentDot, nextDot)
  showHideArrowButtons(nextSlideIndex)
})

We can derive slides from indexes. We can also derive dots from indexes. Why don’t we change all our functions to use index values?

Let’s begin with refactoring switchSlides once more.

Using index-values in switchSlides

If we use index value in switchSlides, we would expect two values:

  1. index of the current slide
  2. index of the target slide
const switchSlide = (currentSlideIndex, targetSlideIndex) => {
  const destination = getComputedStyle(targetSlide).left
  contents.style.transform = `translateX(-${destination})`
  currentSlide.classList.remove('is-selected')
  targetSlide.classList.add('is-selected')
}

We need to find currentSlide and targetSlide from currentSlideIndex and targetSlideIndex.

const switchSlide = (currentSlideIndex, targetSlideIndex) => {
  const currentSlide = slides[currentSlideIndex]
  const targetSlide = slides[targetSlideIndex]
  const destination = getComputedStyle(targetSlide).left

  contents.style.transform = `translateX(-${destination})`
  currentSlide.classList.remove('is-selected')
  targetSlide.classList.add('is-selected')
}

Using the new switchSlide:

When we use the new switchSlide in nextButton's event listener, we no longer need to find nextSlide. Here, we can get currentSlide's index value with findIndex.

nextSlideIndex will be currentSlideIndex + 1

nextButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const nextSlideIndex = currentSlideIndex + 1

  const currentDot = dotsContainer.querySelector('.is-selected')
  const nextDot = currentDot.nextElementSibling

  switchSlide(currentSlideIndex, nextSlideIndex)
  highlightDot(currentDot, nextDot)
  showHideArrowButtons(nextSlideIndex)
})

Remember to change how you use switchSlide in previousButton's and dotsContainer's event listeners:

// In previousButton's event listener
previousButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const previousSlideIndex = currentSlideIndex - 1

  const currentDot = dotsContainer.querySelector('.is-selected')
  const previousDot = currentDot.previousElementSibling

  switchSlide(currentSlideIndex, previousSlideIndex)
  highlightDot(currentDot, previousDot)
  showHideArrowButtons(previousSlideIndex)
})

// In dotsContainer's event listener
dotsContainer.addEventListener('click', event => {
  const dot = event.target.closest('button')
  if (!dot) return

  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const currentDot = dotsContainer.querySelector('.is-selected')
  const targetSlideIndex = dots.findIndex(d => d === dot)

  switchSlide(currentSlideIndex, targetSlideIndex)
  highlightDot(currentDot, dot)
  showHideArrowButtons(targetSlideIndex)
})

Using index values in highlightDot

Here’s what we wrote for highlightDot.

const highlightDot = (currentDot, targetDot) => {
  currentDot.classList.remove('is-selected')
  targetDot.classList.add('is-selected')
}

We know the index value for currentDot is the same as currentSlideIndex. We also know the index value for targetDot is the same as targetSlideIndex. This is true for all three event listeners.

We can replace currentDot and targetDot inside hightlightDot if we find the dots using the indexes.

const highlightDot = (currentSlideIndex, targetSlideIndex) => {
  const currentDot = dots[currentSlideIndex]
  const targetDot = dots[targetSlideIndex]
  currentDot.classList.remove('is-selected')
  targetDot.classList.add('is-selected')
}

Using the new highlightDot:

// In nextButton's event listener
nextButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const nextSlideIndex = currentSlideIndex + 1

  switchSlide(currentSlideIndex, nextSlideIndex)
  highlightDot(currentSlideIndex, nextSlideIndex)
  showHideArrowButtons(nextSlideIndex)
})

// In previousButton's event listener
previousButton.addEventListener('click', event => {
  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const previousSlideIndex = currentSlideIndex - 1

  switchSlide(currentSlideIndex, previousSlideIndex)
  highlightDot(currentSlideIndex, previousSlideIndex)
  showHideArrowButtons(previousSlideIndex)
})

// In dotsContainer's event listener
dotsContainer.addEventListener('click', event => {
  const dot = event.target.closest('button')
  if (!dot) return

  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  const targetSlideIndex = dots.findIndex(d => d === dot)

  switchSlide(currentSlideIndex, targetSlideIndex)
  highlightDot(currentSlideIndex, targetSlideIndex)
  showHideArrowButtons(targetSlideIndex)
})

One last improvement

There’s one last improvement we can make. Notice all three event handlers use this code to get the currentSlideIndex?

const currentSlide = contents.querySelector('.is-selected')
const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)

Guess what? We can put these two lines of code into a dedicated function.

const getCurrentSlideIndex = _ => {
  const currentSlide = contents.querySelector('.is-selected')
  const currentSlideIndex = slides.findIndex(slide => slide === currentSlide)
  return currentSlideIndex
}

If you like it to be shorter, you can return currentSlideIndex without defining it:

const getCurrentSlideIndex = _ => {
  const currentSlide = contents.querySelector('.is-selected')
  return slides.findIndex(slide => slide === currentSlide)
}

Using getCurrentSlideIndex:

// In nextButton's event listener
nextButton.addEventListener('click', event => {
  const currentSlideIndex = getCurrentSlideIndex()
  const nextSlideIndex = currentSlideIndex + 1

  switchSlide(currentSlideIndex, nextSlideIndex)
  highlightDot(currentSlideIndex, nextSlideIndex)
  showHideArrowButtons(nextSlideIndex)
})

// In previousButton's event listener
previousButton.addEventListener('click', event => {
  const currentSlideIndex = getCurrentSlideIndex()
  const previousSlideIndex = currentSlideIndex - 1

  switchSlide(currentSlideIndex, previousSlideIndex)
  highlightDot(currentSlideIndex, previousSlideIndex)
  showHideArrowButtons(previousSlideIndex)
})

// In dotsContainer's event listener
dotsContainer.addEventListener('click', event => {
  const dot = event.target.closest('button')
  if (!dot) return

  const currentSlideIndex = getCurrentSlideIndex()
  const targetSlideIndex = dots.findIndex(d => d === dot)

  switchSlide(currentSlideIndex, targetSlideIndex)
  highlightDot(currentSlideIndex, targetSlideIndex)
  showHideArrowButtons(targetSlideIndex)
})

Once we use indexes consistently in the codebase, our code became much easier to read and understand compared to before!