CODEWITHSUNDEEP
javascriptjquery
P Bateman
P Bateman

Reputation: 3

How can I make this code efficient by looping?

I have am trying to extract an image to display as a background image on my li elements.

The exact number of li's is undefined but my code is not efficient in handling this as I have had to manually write these in. I am not sure the best way to loop through this so that it does not require manual process when more li's are added.

$('.carousel').each(function(){ 

    var i = 0; 
    $(this).find("li").each(function(){
        i++;
        $(this).addClass('thumbnail'+i); 


    });  

    $(this).find("li.thumbnail1").css('background-image', 'url(' + $(this).find('.item:nth-child(1) img').attr('src') + ')');
    $(this).find("li.thumbnail2").css('background-image', 'url(' + $(this).find('.item:nth-child(2) img').attr('src') + ')');
    $(this).find("li.thumbnail3").css('background-image', 'url(' + $(this).find('.item:nth-child(3) img').attr('src') + ')');
    $(this).find("li.thumbnail4").css('background-image', 'url(' + $(this).find('.item:nth-child(4) img').attr('src') + ')');
    $(this).find("li.thumbnail5").css('background-image', 'url(' + $(this).find('.item:nth-child(5) img').attr('src') + ')');

});

Upvotes: 0

Views: 62

Answers (3)

TKoL
TKoL

Reputation: 13902

var that = this;
$(this).find("li").each(function(index){
    var imgSrc = $(that).find('.item:nth-child(' + (index+1) + ') img').attr('src');
    $(this).css('background-image', 'url(' + imgSrc + ')');
});

I think this makes more sense than what you're doing. The jquery .each function has an Index as the first argument so you can find the matching .item with the same index if that's what you need

Upvotes: 1

Justin Pearce
Justin Pearce

Reputation: 5097

Rewriting it like so is a bit more efficient.

$('.carousel').each(function(){ 
    var $carousel = $(this);
    $carousel.find("li").each(function(index){
        $(this).addClass('thumbnail'+index); 
        $(this).css('background-image', 'url(' + $carousel.find('.item:nth-child(index) img').attr('src') + ')');
    });  
});

Storing your carousel the scope of it's each() allows you to interact with it in the scope of your li's each(). There is no need to declare a separate i variable, as the first argument passed to the function in the call to each() is the index of the current item.

Upvotes: 0

Ives Iure M. Ancelmo
Ives Iure M. Ancelmo

Reputation: 1

When you use the $.each() function, you can pass some args to it. For example:

$(this).find("li").each(function(i, o){ });

Where i is the index, and o is the object itself (in your case, the li element). So, you could try doing this:

$(this).find("li.thumbnail" + i).css('background-image', 'url(' + $(this).find('.item:nth-child('+ (i - 1) + ') img').attr('src') + ')');

Upvotes: 0

Related Questions