I have created a jQuery slider for my personal website and I wanted to get some feedback on how it looks and if there are things I can do to make it better.You can see the live demo here: http://www.nickmeagher.comAnd here is the code:        var sliderWidth = 0,        marginLeft = 0,        containerWidth = 1040,        slideSpeed = 600,        slideDuration = 6000,        autoSlide = 0; // (0) = off (1) = onfunction slide_right() {        if ( (sliderWidth - Math.abs(marginLeft)) > containerWidth ) {           $('.portfolio-inner-wrapper').stop().animate({            'margin-left': '+=-' + containerWidth           }, slideSpeed, function() {            marginLeft += -containerWidth;        });     } else {        $('.portfolio-inner-wrapper').stop().animate({            'margin-left': '0px'        }, slideSpeed, function() {            marginLeft = 0;        });    }}function slide_left() {    if ( (sliderWidth - Math.abs(marginLeft)) < containerWidth ) {            $('.portfolio-inner-wrapper').animate({                'margin-left': '+=+' + containerWidth            }, slideSpeed, function() {                marginLeft += containerWidth;            });    } else {        $('.portfolio-inner-wrapper').animate({                'margin-left': '0'            }, slideSpeed, function() {                marginLeft = 0;            });    }}// Iterate through each container to calculate width$('.work-container').each(function() {    sliderWidth += $(this).outerWidth( true );});// Set width of inner wrapper$('.portfolio-inner-wrapper').css('width', sliderWidth);$('a.next').click(function() {    slide_right();});$('a.prev').click(function() {    slide_left();});if (autoSlide == 1) {    setInterval(function() {        slide_right();    }, slideDuration);}
A review of my jQuery Slider
javascript;jquery
Here are some general things you should be doing differently. These suggestions don't change the functionality of your slider, and are suggestions you can apply to your future projects.First and most importantvar wrapper = $('.portfolio-inner-wrapper');//As a rule of thumb, if you use a selector more than once, you should cache it.    //Now use 'wrapper' everywhere you used $('portfilo...') instead.    //Cache it outside if you want to use it in other functions as well.//Ex:wrapper.css('width', sliderWidth);Next:if (autoSlide) { //Don't need == 1. Zero is a falsy value.    setInterval(function() {        slide_right();    }, slideDuration); //You don't need setInterval either. jQuery has a built in queue.}When you have multiple 'animate' calls, jQuery puts them in a queue to execute them one by one. This goes for the rest of your slider as well.Save function calls. Do it like this:$('a.next').on('click', function() {        if ( (sliderWidth - Math.abs(marginLeft)) > containerWidth ) {           wrapper.stop().animate({            'margin-left': '+=-' + containerWidth           }, slideSpeed, function() {            marginLeft += -containerWidth;        });     } else {        wrapper.stop().animate({            'margin-left': '0' //Zero doesn't need a unit of measure (ie. px, em, %, etc)        }, slideSpeed, function() {            marginLeft = 0;        });    }});When you use the .click() method, jQuery calls the .on() method. Then if you still call another function inside that, that's three function calls that could be spared if you simply used this:$('a').on('click', function() {    do.stuff});Finally, I'll be happy to answer any other questions you might have.