Javascript onclick shows last element of array with for loop

Question

I have little problem. I am working with one companys API's. I request for activities and they return me array of activities. Here is part of code.

client.requestActivities(function(activities) {
        if (activities.length > 0) {
            for(i=0; i < activities.length; i++) {
                var activity = activities[i];
                activity.onStart(function() { alert(i+ " started"); });
                activity.onCredit(function() { alert(i+ " credit"); });
                activity.onClose(function() { alert(i+ " close"); });
                activity.onFinish(function() { alert(i+ " finish"); });
                            $('.Game-Screen-Footer').append('<div class="Game-Screen-Footer-Adv" id="foota'+i+'"><a href="javascript:;" ><img src="'+activity.image_url+'" alt="'+activity.display_text+'" width="190" height="110"></a></div>');
                document.getElementById('foota'+i+'').onclick = function() {
                ARNO.box.show({html:'<div id="socialVibeFancyBox0"></div>', close: false, width:activity.window_width, height:activity.window_height, openjs: 
                            function(){
                                client.loadActivityIntoContainer(activity, 'socialVibeFancyBox0');
                            }
                        });
            }
        }

}

Mostly, this function give back array with 3 elements. But when I click on first activity, onStart it should alert me (0 start), but it alerts (4 start), this means, that it alerts last element of i. How should I fix this? I tried a lot of stuff, but I haven't find solution, maybe you can help me?

Niet the Dark Absol · Accepted Answer · 2012-10-03 23:27:41Z

5

This is yet another case of closure problems. Try this:

for( i=0; i<activities.length; i++) {
    (function(i) {
        // your code here
    })(i);
}

answered Oct 3, 2012 at 23:27

Niet the Dark Absol

326k86 gold badges480 silver badges604 bronze badges

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

1 Comment

hakre Over a year ago

@Kolnik: That sounds interesting. Do you have to a reference Q&A which explains this closure problems in detail at hand?

I Hate Lazy · Accepted Answer · 2012-10-03 23:30:31Z

Move the guts of the for loop out to a function, and pass in the values that need to be scoped separately.

for(i=0; i < activities.length; i++) {
    setUpActivity(i, activities[i]);
}

function setUpActivity(i, activity) {

    activity.onStart(function() { alert(i+ " started"); });
    activity.onCredit(function() { alert(i+ " credit"); });
    activity.onClose(function() { alert(i+ " close"); });
    activity.onFinish(function() { alert(i+ " finish"); });
    ARNO.box.show({
        html:'<div id="socialVibeFancyBox0"></div>', 
        close: false, width:activity.window_width, 
        height:activity.window_height, 
        openjs:function(){
                    client.loadActivityIntoContainer(activity, 'socialVibeFancyBox0');
                }
            });
}

A function invocation creates a new variable scope. any variables or parameters created in the function will be in their own scope, and the functions created in the same scope will have permanent reference to those variables.

elclanrs · Accepted Answer · 2012-10-03 23:32:45Z

0

If you don't care about IE8- then ES5 already solves this issue with forEach:

activities.forEach(function(v, i){
  // now 'i' will work anywhere within this scope
});

answered Oct 3, 2012 at 23:32

elclanrs

94.2k21 gold badges137 silver badges171 bronze badges

Collectives™ on Stack Overflow

Javascript onclick shows last element of array with for loop

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related