Delegating events by parsing strings in a for loop

Question

Possible Duplicate:
Javascript closure inside loops - simple practical example

Recently I have been playing around with some of the more complicated aspects of JavaScript, one of which is design patterns and delegation.

I have created a object containing 2 keys, which I then parse and attach an event onto the body.

My problem is that it seems like the last function is attached for any iteration of the for loop. I am pretty sure this is a copy by reference/scoping(thanks Shmiddty) problem, but please correct me if I am wrong.

Is there anyway to fix my issue to attach the 2 events on properly without changing too much? (As this is a complex app that I have simplified down to the issue at hand).

On the jsfiddle example please see how the alert message for .div1 is the same as .div2

var foo = {
    init: function() {

        var i = 0,
            keys = []
            self = this;

        for (var key in this) {
            keys.push(key);
        }

        for (i; i < keys.length; i++) {
            if (keys[i].match(/^on/i)) {
                var delegateFunc = keys[i].split(' | '),
                    event = delegateFunc[2],
                    selector = delegateFunc[1],
                    keyName = keys[i];

                console.log('###');
                console.log( 'selector: ' + selector );
                console.log( 'event: ' + event );
                console.log( 'function: ' + self[keyName] );

                $('body').on(event, selector, function(ev) {
                    self[keyName](ev, ev.target);
                });
            }
        }

    },

    'on | .div1 | click': function(ev, el) {
        alert('you clicked 1');
    },

    'on | .div2 | click': function(ev, el) {
        alert('you clicked 2');
    }
}

$(function() { foo.init(); });

Any more info needed? Leave me a comment. Cheers

@Shmiddty Could you please explain what you mean rather than put a joking comment on my question? — Undefined, Jan 04 '13 at 00:18
I mean that this question is asked roughly a dozen times a day. — Shmiddty, Jan 04 '13 at 00:20

ThiefMaster · Accepted Answer · 2013-01-04T00:28:48.730

2

            $('body').on(event, selector, function(ev) {
                self[keyName](ev, ev.target);
            });

replace this with the following code:

            (function(keyName) {
                $('body').on(event, selector, function(ev) {
                    self[keyName](ev, ev.target);
                });
            })(keyName);

The reason for this is that JavaScript doesn't have a block scope. So your var keyName is the same variable in each loop and the closure of your callback function binds the variable, not the value it has at that moment. So as soon as the loop finished the variable will be whatever its last value was in all your callback functions.

By adding the immediately-executed function which takes keyName as an argument you create a new variable (the function argument) and thus avoid the issue.

edited Jan 04 '13 at 00:28

answered Jan 04 '13 at 00:21

ThiefMaster

310,957
84
592
636

Thank you @ThiefMaster your answer was the quickest/easiest to understand answer of them all. I'll mark as accepted once I can – Undefined Jan 04 '13 at 00:23
"self-executing" is a misleading term. "Immediately Invoked" is more accurate, since the function isn't self-aware. Or has skynet become self-aware? – Shmiddty Jan 04 '13 at 00:26

score 1 · Answer 2 · answered Jan 04 '13 at 00:21

Classic closure issue..

Put your for loop in an immediately invoked function expression

for (i; i < keys.length; i++) {
            (function(lockedIndex) {
                if (keys[lockedIndex].match(/^on/i)) {
                    var delegateFunc = keys[lockedIndex].split(' | '),
                        event = delegateFunc[2],
                        selector = delegateFunc[1],
                        keyName = keys[lockedIndex];

                    console.log('###');
                    console.log('selector: ' + selector);
                    console.log('event: ' + event);
                    console.log('function: ' + self[keyName]);

                    $('body').on(event, selector, function(ev) {
                        self[keyName](ev, ev.target);
                    });
                }
            })(i)
        }

Check Fiddle

He doesn't even need to lock the index in his case. `keyName` was the culprit. — ThiefMaster, Jan 04 '13 at 00:22
Thanks for the link for me to read through, have an upvote :) — Undefined, Jan 04 '13 at 00:23

score 0 · Answer 3 · answered Jan 04 '13 at 00:21

0

keyName is stale by the time your event handler is triggered, you need to wrap it in a new scope:

(function(key) {
    $('body').on(event, selector, function(ev) {
        self[key](ev, ev.target);
    });
})(keyName);

answered Jan 04 '13 at 00:21

Shmiddty

13,847
1
35
52

Delegating events by parsing strings in a for loop

3 Answers3