Detected loop counter variable value is wrong

Question

My test code:

var imageUploadPreview = function (e) {
  var input = document.getElementById("file"),
      files = e.target.files;

    for (var i = 0; i < files.length; i++) {
        if (input.files) {
            var file = files[i],
                reader = new FileReader();

            reader.onload = function (e) {
                (function (i) {
                    var img = $('<img>').prop('id', 'img_' + i)
                     .prop('src', e.target.result).css({'width':'60px', 'height':'60px'});

                    var div = $('<div>').append(img).append(img.prop('id'))
                    $('body').append(div);
                })(i)
            }
            reader.readAsDataURL(file);
        }
    }
};

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>


<input type="file" id="file" onchange="imageUploadPreview(event)" multiple />

It's working except: all image ids are img_ + files.length

Can you explain me why? What's happen with the loop?

Possible Duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Tushar, Dec 10 '15 at 13:44
It is not a normal dupe of the closure question, OP is already doing it.... Just the wrong place. — epascarello, Dec 10 '15 at 13:47

epascarello · Accepted Answer · 2015-12-10T13:58:36.343

3

You almost had it with the iife executing i, you need to move it outside of the onload. The problem is the onload is called asynchronously, so by the time it has run the loop is finished. That is why i is the last index for every file. You need to execute the iife before the onload runs.

        reader.onload = function (e) {
            (function (i) {
                var img = $('<img>').prop('id', 'img_' + i)
                 .prop('src', e.target.result).css({'width':'60px', 'height':'60px'});

                var div = $('<div>').append(img).append(img.prop('id'))
                $('body').append(div);
            })(i)
        }

to

(function (i) {
    reader.onload = function (e) {
        var img = $('<img>').prop('id', 'img_' + i)
            .prop('src', e.target.result).css({
                'width': '60px',
                'height': '60px'
            });

        var div = $('<div>').append(img).append(img.prop('id'))
        $('body').append(div);
    }
})(i);

edited Dec 10 '15 at 13:58

answered Dec 10 '15 at 13:46

epascarello

204,599
20
195
236

_So you need to execute it before the onload runs_ for this one +1. – Jai Dec 10 '15 at 13:50
Sorry. Can I understand that `move iife ouside of the onload` mean: stop looping until the onload finished? – Tân Dec 10 '15 at 13:54
1

No, look at what I did I took the iife aka `(function(i){})(i)` that was inside of the onload and instead wrapped it. When your code runs, that function is not executed until after the loop. In my version, it is executed whwn the loop runs which than will hold the correct value of the `i` you want. – epascarello Dec 10 '15 at 13:56
@epascarello Naming of the argument too as `i` can confuse the OP – Charlie Dec 10 '15 at 14:00

score 2 · Answer 2 · answered Dec 10 '15 at 13:54

2

Your call to reader.readAsDataURL(file); is asynchronous.

This means that reader.onload function doesn't execute with the loop synchronously. So, the value of the loop variable is unpredictable when the load function executes.

Encapsulate your reader.onload with another function after passing the loop variable to it.

for(i) {
   function(loopVar) {
      reader.onload = function () {
          //use loopVar here
      }(i)
}

Here you register your loop variable i as an argument to the encapsulating function. The value, as of the time it has been passed, will still remain whenever your callback function is executed.

answered Dec 10 '15 at 13:54

Charlie

22,886
11
59
90

Sound like creating anonymous function and wating for callback to continue looping. – Tân Dec 10 '15 at 14:04
1

Yes - you create an anonymous function. No - it doesn't wait. It executes immediately with a specific value in it's argument. This value remains the same until the callback is executed. – Charlie Dec 10 '15 at 14:07

Detected loop counter variable value is wrong

2 Answers2