javascript closure in for loop not working

Question

Possible Duplicate:
Javascript closure inside loops - simple practical example

I'm having a problem with a closure in a phonegap application. I have a JSON object which contains instructions on how to create a form. This is roughly the code:

for (var i in this.form.elements) {
    var element = this.form.elements[i];
    switch (element.type) {
        // other cases
        case 5:
            var addThumb, photoInput;
            addThumb = function (domId) {
                return function (imgData) {
                    console.log('id: ' + domId);
                    $(domId).attr({src: 'data:image/jpeg;base64,' + imgData});
                }
            }('#thumb-'+element.id);
            photoInput = $('<input>')
                .attr({type: 'button', value: element.name, name: element.type, id: element.id, 'class': 'photobutton'})
                .click(function (filename, cb) {
                    return function() {
                        console.log('taking photo and saving to ' + filename)
                        takePhoto(filename,addThumb)
                    }
                }('photo-'+element.id, addThumb)
            );
            photoThumb = $('<img>').attr({id: 'thumb-'+element.id, 'class': 'thumbnail', src: 'img/target.png'});
            $('#content').append(photoInput, photoThumb);
            break;
    }
}

Clicking the photobutton opens up a camera to take a picture, which is saved to disk. Once saved, addThumb is called with the base64 photo data to replace the thumbnail image. The output for each for my test form is:

taking photo and saving to file photo-686
id: '#thumb-690

The closure for taking the photo works, but when addThumb is called as a callback, domId shows the last element.id that went through the for loop. Given two thumbnails, the second one is always replaced regardless of the button pressed. It happens on both iPhone and Android so there must be a problem with the way I've written it. What did I do wrong?

this.form is just an object, no html elements. It has the mapping for what html elements to make and what their ids and values should be, like values for combo boxes. — brodney, Jan 04 '13 at 21:04
I guess `form.elements` is array-like, so you should not use a for-in-loop here. — Bergi, Jan 04 '13 at 21:05

score 2 · Accepted Answer · answered Jan 04 '13 at 21:03

2

takePhoto(filename, addThumb)

You need to pass the cb variable from inside the closure here, not the addThumb which is not "protected". Seems like you forgot to change the variable name when "protecting" it.

answered Jan 04 '13 at 21:03

Bergi

630,263
148
957
1,375

+1 My explication is right, but yours captures OP's intent. It's much better and useful than mine. – Denys Séguret Jan 04 '13 at 21:04
You are correct! Thanks for spotting it, you and @dystroy. – brodney Jan 04 '13 at 21:08

Denys Séguret · Answer 2 · 2013-01-04T21:02:01.120

The problem is that the addThumb variable itself isn't protected by a closure. It encloses the value of element.id at the time it is created but it's replaced in the next occurrence of the loop. So you're always calling the last created addThumb function.

Instead of trying to protect each variable separately, you could build one closure to enclose element :

for (var i in this.form.elements) {
    var element = this.form.elements[i];
    (function(element){
       switch (element.type) {
       ...
    })(element);
}

javascript closure in for loop not working

2 Answers2