Javascript Function not working in for loop

Question

I'm trying to make a javascript code smaller using a for loop, im new to javascript but I thought this would work,

Im trying to make this smaller:

   $("li#li_item1").click(function(){
    all();
    $("div#item1").fadeIn("fast");
})

$("li#li_item2").click(function(){
    all();
    $("div#item2").fadeIn("fast");
})

$("li#li_item3").click(function(){
    all();
    $("div#item3").fadeIn("fast");
})

$("li#li_item4").click(function(){
    all();
    $("div#item4").fadeIn("fast");
})

Using this:

    var AantalItem = 159;

    for(var k=0;k<=AantalItems;k++) {
    $("li#li_item" + k).click(function(){
        all();
        $("div#item" + k).fadeIn("fast");
    })

    document.getElementById("test").innerHTML=k;
}

When using the smaller code the fade in just wont work

This is the all():

    var all = function(){
    for(var i=0;i<=AantalItems;i++) {
        $("div#item" + i).fadeOut("fast");
    }
};

Yeah, 'k' is always === AantalItems-1 when the click handler is executed. But, apart from that, the original code is totally easier to read. — johnnycardy
– johnnycardy, Commented Jan 7, 2014 at 10:33

Zaheer Ahmed · Accepted Answer · 2014-02-02 10:34:59Z

2

It is due to closures effect.

try this:

for(var k=0;k<=AantalItems;k++) {
  $("li#li_item" + k).click(createClickFunction(k));
}

function createClickFunction(k) {
     return function() {
            all();
            $("div#item" + k).fadeIn("fast");
        }
}

edited Feb 2, 2014 at 10:34

answered Jan 7, 2014 at 10:37

Zaheer Ahmed

28.7k12 gold badges77 silver badges113 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user3168692 Over a year ago

Thanks for the fast response! but it didn't seem to do it, it also wont fade out anymore using that hmm

user3168692 Over a year ago

This seems to do it, but it now fades out and then fades back in so it double fades out

Sunny · Accepted Answer · 2014-01-07 10:41:41Z

0

Try this..

  function fadeinAll() {
         $('li#li_item').find('div').each(function () { $(this).fadeIn("fast"); })
     }
     function fadeoutAll() {
         $('li#li_item').find('div').each(function () { $(this).fadeOut("fast"); })
     }

answered Jan 7, 2014 at 10:41

Sunny

2191 silver badge10 bronze badges

Comments

Adassko · Accepted Answer · 2014-01-07 10:48:34Z

you can create a function returning a function and pass k to it:

for(var k=0;k<=AantalItems;k++) {
    $("li#li_item" + k).click((function(k){
        return function() {
            all();
            $("div#item" + k).fadeIn("fast");
        }
    })(k));

or just add a common class to those items, and linked object's id to its data property - then the whole code will look like:

$('.commonClass').click(function() {
    $('.anotherCommonClassForDivs').fadeOut("fast");
    $('div#' + $(this).data('id')).fadeIn("fast");
});

and html:

<li class="commonClass" data-id="item1">whatever</li>
<li class="commonClass" data-id="item2">whatever</li>

<div class="anotherCommonClassForDivs" id="item1">data for item1</div>
<div class="anotherCommonClassForDivs" id="item2">data for item2</div>

LIVE DEMO

Collectives™ on Stack Overflow

Javascript Function not working in for loop

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related