Periodically sliding images

Question

I wrote this code using jQuery, but I think it can be improved. Can you provide some feedback?

    var $i = 1,
    totalImg = $('.home-featured-bg .absolute div').length,
    $theWidth = $(window).width(),
    $theHeight = $(window).height();
    $('.home-featured-bg .absolute').css('width',$theWidth * totalImg+'px');
    $('.home-featured-bg .absolute .slide,.home-featured-bg .absolute .slide img').css({'width':$theWidth+'px','height' : $theHeight+'px'});

window.setInterval(function(){
    $('.events-links a').hide();
    if($i == 1)
    {

        var $el = $theWidth;
    }else{
        var $el = $theWidth * $i;
    }
    if($i < totalImg)
    {
        $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
        $i++;
        $('.events-links a').eq($i - 1).css('display','block');
    }else{
        $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
        $i = 1;
        $('.events-links a').eq($i - 1).css('display','block');
    }


},3000);

$(window).resize(function(){
    totalImg = $('.home-featured-bg .absolute div').length,
    $theWidth = $(window).width(),
    $theHeight = $(window).height();
    $('.home-featured-bg .absolute').css('width',$theWidth * totalImg+'px');
    $('.home-featured-bg .absolute .slide,.home-featured-bg .absolute .slide img').css({'width':$theWidth+'px','height' : $theHeight});
});

Answer 1

Here few suggestion:

in the function you have :

if($i == 1)
{

    var $el = $theWidth;
}else{
    var $el = $theWidth * $i;
}

could be replaced by a single line:

var $el = $theWidth * $i;

and you could modify the following if :

if($i < totalImg)
{
    $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
    $i++;
    $('.events-links a').eq($i - 1).css('display','block');
}else{
    $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
    $i = 1;
    $('.events-links a').eq($i - 1).css('display','block');
}

by extract the $('.events-links a').eq($i - 1).css('display','block'); like this:

if($i < totalImg)
{
    $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
    $i++;
}else{
    $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
    $i = 1;
}
$('.events-links a').eq($i - 1).css('display','block');

Answer 2

I agree with Nettogrof on the following:

var $el = $theWidth * $i;

But you can shorten your code even more by doing the following (*not tested):

var posNeg = '-';
if($i < totalImg) {$i = 0; posNeg='';}
$('.home-featured-bg .absolute').animate({marginLeft:posNeg +$el+'px'},2000);
$('.events-links a').eq($i).show();
$i++;

More things you can do to improve code:

Set variables for selector objects, makes changes easier, and easier to manage etc., e.g:

Instead of :

$('.myClass').fadeIn();
$('.myClass').css('color','#ff0000');

Use :

var mySel = $('.myClass');
mySel.fadeIn().css('color','#ff0000');