8

I have the following code which runs in a loop:

var index = fileNames[x].lastIndexOf("/") + 1;
var currentImageName = fileNames[x].substr(index);

if (currentImageName.indexOf(".jpg") != -1) {
reader.getFileAsBlob(fileNames[x])
    .done(function(blob) {
        picturesFilePathArray.push({
           fileName: currentImageName,
           fileURL: blobURL(blob)
        });
       refreshKMZList();
    });
}

The problem I am having is that I am trying to save an object with 2 properties into an array. This object should have the identifier and the result. (fileName and fileURL respectively). But since this function is asynchronous (executed through a promise). By the time the "getFileAsBlob" finishes, currentImageName has already been updated ultimately ending in many of my objects having the same identifier (the last one processed before it finished).

This might be a really easy problem, but I am very new to javascript and haven't yet found anything about it.

I thought that the solution might be in passing the variable to the "done" function, but I think this function is the one being returned by the method and is already set. (I dont know how it looks)

Edit:

The code is just inside a normal loop

for (x = 0; x<fileNames.length; x++)
Pochi
  • 13,391
  • 3
  • 64
  • 104

2 Answers2

9

So create a function so the variable can not be changed

function getFile (filmName, imageName) {
    reader.getFileAsBlob(fileName)
    .done(function(blob) {
        picturesFilePathArray.push({
           fileName: imageName,
           fileURL: blobURL(blob)
        });
       refreshKMZList();
    });
}

and call it

if (currentImageName.indexOf(".jpg") != -1) {
    getFile (fileNames[x], currentImageName);
}

or you can do something like

if (currentImageName.indexOf(".jpg") != -1) {
    (function (fileName, imageName) { 
    reader.getFileAsBlob(fileName)
        .done(function(blob) {
            picturesFilePathArray.push({
               fileName: imageName,
               fileURL: blobURL(blob)
            });
           refreshKMZList();
        });
    })(fileNames[x], currentImageName);
}

MDN Closure

epascarello
  • 204,599
  • 20
  • 195
  • 236
4

The solution to this problem is always the same: Use a closure.

But since you are using a promise based library, you have a nicer option. Use promises. (Internally this is based on closures as well, of course. It's just a much nicer abstraction.)

function getFileInfo(path) {
    return reader.getFileAsBlob(path).done(function (blob) {
        return {
            fileName: path.split('/').pop(),
            fileURL: blobURL(blob)
        });
    };
}

function isJpg(filename) {
    return /\.jpg$/i.test(filename);
}

Now you can do this, where refreshKMZList() is called once per file:

fileNames.filter(isJpg).forEach(function (path) {
    getFileInfo(path).then(function (fileInfo) {
        picturesFilePathArray.push(fileInfo);
        refreshKMZList();
    })
    .catch(function (error) {
        // handle the error
    });
});

or even this, where refreshKMZList() is called only once per overall:

var fileInfos = fileNames.filter(isJpg).map(getFileInfo);

Promise.all(fileInfos).then(function (arrayOfFileInfo) {
    picturesFilePathArray.concat(arrayOfFileInfo);
    refreshKMZList();
})
.catch(function (error) {
    // handle the error
});

Read up on promises, they are worth being understood.

Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • thanks for your reply, this loos very elegant. I got a couple of questions tho, this "/\.jpg$/i.test(filename)" tests for a file with jpg ending i suppose, but how can i search for an explanation of this type of syntax? (in google, how is it called). and is it possible to add other file types? like jpeg, png, gif? and second, for the solution in where refreshKMZList() is run once, isnt .all IE specific? and am i supposed to write it like that? Promise.all(FileNames.map... ? Thanks! – Pochi Jan 16 '14 at 00:52
  • Oh, by your questions I see that you *really* are at a beginner level. There is an awful lot of learning ahead of you. ;) The strange syntax is called a "regular expression". That's a can of worms of its own, it takes a very long time to master. Maybe in your situation it's better to use the same trick I used to pull out the last path segment instead. And `document.all()` is IE specific, `Promise.all()` is something else entirely. (Then again, at the time of writing this promises themselves are [natively supported only in Google Chrome](http://caniuse.com/#feat=promises).) – Tomalak Jan 16 '14 at 02:09
  • haha, yeah i guess i have a long way. Thanks a lot for your help! – Pochi Jan 16 '14 at 02:44