Help me to use DRY principle in my JS code

JavaScript
1

I made a simple slider that switches images back and forth, but I don’t like that I have the same currentActiveItem variable inside two functions. If I place only one variable outside of functions the sliders work only once and stop executing.

Here is my JS code

'use strict'

const SWIPER = document.getElementsByClassName('swiper')
  , SWIPER_CONTENT = document.getElementsByClassName('swiper-content')
  , SWIPER_ITEM = document.querySelectorAll('.swiper-content .item')
  , ARROW_PREV = document.querySelector('.arrow-prev')
  , ARROW_NEXT = document.querySelector('.arrow-next');

function SlideNext() {
    let currentActiveItem = document.querySelector('.swiper-content .item.active')

    if (typeof(currentActiveItem.nextElementSibling) != 'undefined' && currentActiveItem.nextElementSibling != null) {
        currentActiveItem.classList.remove('active');
        currentActiveItem.nextElementSibling.classList.add('active');
    } else {
        currentActiveItem.classList.remove('active');
        SWIPER_ITEM[0].classList.add('active');
    }
}

function SlidePrevious() {
    let currentActiveItem = document.querySelector('.swiper-content .item.active')

    if (typeof(currentActiveItem.previousElementSibling) != 'undefined' && currentActiveItem.previousElementSibling != null) {
        currentActiveItem.classList.remove('active');
        currentActiveItem.previousElementSibling.classList.add('active');
    } else {
        currentActiveItem.classList.remove('active');
        SWIPER_ITEM[SWIPER_ITEM.length -1].classList.add('active');
    }
}

ARROW_NEXT.addEventListener('click', SlideNext);
ARROW_PREV.addEventListener('click', SlidePrevious);

Help me, please :slight_smile:

2

I see redundant code in there.
For example currentActiveItem.classList.remove('active'); you want this to happen in any condition, just remove it from the if-else statement conditions and put it in the SlideNext function scope somewhere outside of else/if (removing the duplicate).

This bit of code must be a mistake SWIPER_ITEM[0].classList.add('active'); , this makes always the first element active, the index 0.

I think you need to do some research to understand how the scope works, how the synchronous code is evaluated (the order) and how to efficiently write conditions.

Maybe would be useful to add in there some console.log statements so you can keep track of the current state of the DOM on each function execution.

I have no idea of the context of your application but this bit of code does look error prone.