CODEWITHSUNDEEP
jqueryperformance
Fusentast
Fusentast

Reputation: 1

Is it possible to optimize my setInterval function a bit?

I have made this simple slider with jQuery and CSS.

https://jsfiddle.net/v39qch7o/2/

var ticker = 0;
var domElement = $('.ticker__item');

window.setInterval(function() {
  ticker = ticker + 1;
  $(domElement).removeClass('active');
  //  console.log(ticker);

  if (ticker == 1) {
    $('.div-1').addClass('active');
  }
  if (ticker == 2) {
    $('.div-2').addClass('active');

  }
  if (ticker == 3) {
    $('.div-3').addClass('active');
    ticker = 0;
  }
}, 2000);
.ticker__item {
  color: #fff;
  padding: 10px;
  position: absolute; 
  top: 0;
  left: 0;
  opacity: 0;
  transition: opacity .2s ease;
}
.div-1 {
  background: pink;
}
.div-2 {
  background: green;
}
.div-3 {
  background: brown;
}
.active {
  opacity: 1;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.0.0/jquery.min.js"></script>
<div class="ticker">
  <div class="ticker__item div-1">Hello world 1</div>
  <div class="ticker__item div-2">Hello world 2</div>
  <div class="ticker__item div-3">Hello world 3</div>
</div>

I'm wondering if is an OK way to make a simple slide show. Will it be OK to run this code when my users are on a phone, and they have scrolled pass these divs. Will it make their devices work to hard and drain their batteries?

I just want to make nice, simple code. Please review my code and tell me if I could do it in an more performance friendly manner :-)

Upvotes: 0

Views: 156

Answers (2)

Rajesh
Rajesh

Reputation: 24925

Since you are looking for performance, you should try to have a unique selector.

$(domElement).removeClass('active'); This will loop and remove class from all divs, but only 1 div has the necessary class, so you have n-1 iterations that will have no effect.

Instead, try $('.ticker__item.active');.

Also, having a count variable and div-1 ... div-3 looks like an overhead. You can just use generic classes and check for next sibling and add class to it. If you do not have next sibling, jump to first sibling and restart the process again.

This removes any dependency on counter variable(ticker) and removes 1 unwanted global variable. This also makes your function generic and if you wish to add more ticker, you just have to add valid markup.

window.setInterval(function() {
  var nextDiv = $('.ticker__item.active')
    .removeClass('active')
    .next('.ticker__item');
  var firstDiv = $('.ticker__item:first');
  (nextDiv.length ? nextDiv : firstDiv).addClass('active'); 
}, 2000);
.ticker__item {
  color: #fff;
  padding: 10px;
  position: absolute; 
  top: 0;
  left: 0;
  opacity: 0;
  transition: opacity .2s ease;
}
.div-1 {
  background: pink;
}
.div-2 {
  background: green;
}
.div-3 {
  background: brown;
}
.active {
  opacity: 1;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.0.0/jquery.min.js"></script>
<div class="ticker">
  <div class="ticker__item div-1">Hello world 1</div>
  <div class="ticker__item div-2">Hello world 2</div>
  <div class="ticker__item div-3">Hello world 3</div>
</div>

Upvotes: 0

tymeJV
tymeJV

Reputation: 104775

You can use modulo math and just concat the ticker variable:

window.setInterval(
function () {
    domElement.removeClass('active');
    $('.div-' + (++ticker)).addClass('active');
    ticker = ticker % 3
}, 2000);

Upvotes: 1

Related Questions