2
\$\begingroup\$

Can anyone optimize this code? I am new to jQuery.

I want add class on next and previous button click. I wrote this code that works for me, but could anyone optimize it using jQuery predefined methods? That would be helpful.

$(document).ready(function () {
    var length = $('#slides li').size() - 1;
    var curren = 0;
    console.log(length);
    $('.next').on('click', function () {
        if (curren >= 0 && curren < length) {
            curren++;
            $('.selected').removeClass('selected');
            $('#slides li:eq(' + curren + ')').addClass('selected');
        }
    });
    $('.prev').on('click', function () {
        if (curren >= 1) {
            curren--;
            $('.selected').removeClass('selected');
            $('#slides li:eq(' + curren + ')').addClass('selected');
        }
    });
});

My HTML code:

<ul id="slides">
    <li class="selected">first</li>
    <li>second</li>
    <li>third</li>
    <li>fourth</li>
    <li>five</li>
    <li>six</li>
</ul>

CSS code:

.selected{
color:red;
}
\$\endgroup\$
1
\$\begingroup\$

There's not much in the way of predefined jQuery methods to add. Here's what I came up with though...

$(document).ready(function () {
    var $slides = $('#slides li'); //Save repeated selector result. 
    var length = $slides.length - 1; //size is deprecated
    var curren = 0;
    console.log(length);
    // used .click due to personal preference .on('click', does the same thing
    $('.next').click(function () {
        if (curren < length) { //Removed check that will always be true
            curren++;
            changeSelected(curren); //Moved repeated code to function
        }
    });
    $('.prev').click(function () {
        if (curren >= 1) {
            curren--;
            changeSelected(curren);
        }
    });

    function changeSelected(index) {
        $('.selected').removeClass('selected');
        $slides.eq(index).addClass('selected'); //Use previously selected object
    }
});

Here's a link to the doc for .size() where you can find it is deprecated.

\$\endgroup\$
  • \$\begingroup\$ FYI, I didn't know about .next() and .prev(), Flambino's answer better suits your requirements and is niftier. \$\endgroup\$ – Daniel Nov 7 '13 at 14:36
1
\$\begingroup\$

See this very similar question and its answers (it's slightly different in that it loops if you keep clicking next or previous, but otherwise it's the same idea).

Here's my take on a (non-looping) version

$(function () { // same as (document).ready(function () {..})
  var slides = $("#slides li"); // find the slides once

  // common next/prev function
  function changeSlide(direction) {
    var target,
        current = slides.filter(".selected"); // find the current slide
    target = current[direction]();            // call either .next() or .prev()
    if(target.length) {                       // if there is a next/prev slide switch to it
      current.removeClass();
      target.addClass("selected");
    }
  }

  // add the handlers
  $(".next").on("click", function () { changeSlide('next') });
  $(".prev").on("click", function () { changeSlide('prev') });
});

Here's a demo

\$\endgroup\$
  • \$\begingroup\$ Very nice. Unfortunately, the buttons don't do anything in my browser, IE 9.0. I wonder why... \$\endgroup\$ – Daniel Nov 7 '13 at 13:56
  • 1
    \$\begingroup\$ @DanielCook Ugh... I'd love to say I'm surprised, but nothing about IE can really surprise me anymore. I have no idea what might cause it (and I don't have a Windows machine handy to check). But it works fine in Chrome/FF/Safari/Opera and on my phone... \$\endgroup\$ – Flambino Nov 7 '13 at 14:15
  • \$\begingroup\$ Naturally, I'm tempted to ask on SO. It certainly looks like it should work. +1 \$\endgroup\$ – Daniel Nov 7 '13 at 14:37
  • \$\begingroup\$ Thought you'd like to know that this DOES work on IE 9.0. It just that your demo doesn't work on IE 9.0. I copied this independently outside of jsfiddle and it worked just fine. \$\endgroup\$ – Daniel Nov 7 '13 at 14:49
  • \$\begingroup\$ @DanielCook Well that's a relief. Still pretty weird, though. Thanks for testing it! \$\endgroup\$ – Flambino Nov 7 '13 at 15:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.