Menu traversing on next and prev button click up and down

Question

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;
}

Answer 1

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.

• Daniel: FYI, I didn't know about .next() and .prev(), Flambino's answer better suits your requirements and is niftier.

Answer 2

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

• Daniel: Very nice. Unfortunately, the buttons don't do anything in my browser, IE 9.0. I wonder why...
• Flambino: @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...
• Daniel: Naturally, I'm tempted to ask on SO. It certainly looks like it should work. +1
• Daniel: 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.
• Flambino: @DanielCook Well that's a relief. Still pretty weird, though. Thanks for testing it!