23

Note: This question is also cross-posted in Q.js mailing list over here.


i had a situation with multiple asynchronous operations and the answer I accepted pointed out that using Promises using a library such as q.js would be more beneficial.

I am convinced to refactor my code to use Promises but because the code is pretty long, i have trimmed the irrelevant portions and exported the crucial parts into a separate repo.

The repo is here and the most important file is this.

The requirement is that I want pageSizes to be non-empty after traversing all the dragged'n dropped files.

The problem is that the FileAPI operations inside getSizeSettingsFromPage function causes getSizeSettingsFromPage to be async.

So I cannot place checkWhenReady(); like this.

function traverseFiles() {
  for (var i=0, l=pages.length; i<l; i++) {
    getSizeSettingsFromPage(pages[i], calculateRatio);   
  }
  checkWhenReady(); // this always returns 0.
}

This works, but it is not ideal. I prefer to call checkWhenReady just ONCE after all the pages have undergone this function calculateRatio successfully.

function calculateRatio(width, height, filename) {
  // .... code 
  pageSizes.add(filename, object);
  checkWhenReady(); // this works but it is not ideal. I prefer to call this method AFTER all the `pages` have undergone calculateRatio
  // ..... more code...
}

How do I refactor the code to make use of Promises in Q.js?

Community
  • 1
  • 1
Kim Stacks
  • 10,202
  • 35
  • 151
  • 282

2 Answers2

45

My suggestions to get this working with Q.js are below. The key is that anytime you want to do something asynchronously, you should return a promise, and once the task is completed you should resolve that promise. That allows the callers of the function to listen for the task to be completed and then do something else.

As before, I have commented my changes with // ***. Let me know if you have any further questions.

        function traverseFiles() {
            // *** Create an array to hold our promises
            var promises = [ ];
            for (var i=0, l=pages.length; i<l; i++) {
                // *** Store the promise returned by getSizeSettingsFromPage in a variable
                promise = getSizeSettingsFromPage(pages[i]);
                promise.then(function(values) {
                    var width = values[0],
                        height = values[1],
                        filename = values[2];
                    // *** When the promise is resolved, call calculateRatio
                    calculateRatio(width, height, filename);
                });
                // *** Add the promise returned by getSizeSettingsFromPage to the array
                promises.push(promise);
            }
            // *** Call checkWhenReady after all promises have been resolved
            Q.all(promises).then(checkWhenReady);
        }

        function getSizeSettingsFromPage(file) {
            // *** Create a Deferred
            var deferred = Q.defer();
            reader = new FileReader();
            reader.onload = function(evt) {
                var image = new Image();
                image.onload = function(evt) {
                    var width = this.width;
                    var height = this.height;
                    var filename = file.name;
                    // *** Resolve the Deferred
                    deferred.resolve([ width, height, filename ]);
                };
                image.src = evt.target.result;
            };
            reader.readAsDataURL(file);
            // *** Return a Promise
            return deferred.promise;
        }

Edit

defer creates a Deferred, which contains two parts, a promise and the resolve function. The promise is returned by getSizeSettingsFromPage. Basically returning a promise is a way for a function to say "I'll get back to you later." Once the function has completed it's task (in this case once the image.onload event has fired) the resolve function is used to resolve the promise. That indicates to anything waiting on the promise that the task has been completed.

Here's a simpler example:

function addAsync(a, b) {
    var deferred = Q.defer();
    // Wait 2 seconds and then add a + b
    setTimeout(function() {
        deferred.resolve(a + b);
    }, 2000);
    return deferred.promise;
}

addAsync(3, 4).then(function(result) {
    console.log(result);
});
// logs 7 after 2 seconds

The addAsync function adds two numbers but it waits 2 seconds before adding them. Since it's asynchronous, it returns a promise (deferred.promse) and resolves the promise after the 2 second wait (deferred.resolve). The then method can be called on a promise and passed a callback function to be executed after the promise has been resolved. The callback function is passed in the resolution value of the promise.

In your case, we had an array of promises and we needed to wait for all of them to be done before executing a function, so we used Q.all. Here's an example:

function addAsync(a, b) {
    var deferred = Q.defer();
    // Wait 2 seconds and then add a + b
    setTimeout(function() {
        deferred.resolve(a + b);
    }, 2000);
    return deferred.promise;
}

Q.all([
    addAsync(1, 1),
    addAsync(2, 2),
    addAsync(3, 3)
]).spread(function(result1, result2, result3) {
    console.log(result1, result2, result3);
});
// logs "2 4 6" after approximately 2 seconds
Nathan Wall
  • 10,530
  • 4
  • 24
  • 47
  • Your code works, Nathan. Because I want to learn, i want to ask, why is it that you use defer in getSizeSettngsFromPage? The answer provided by missingno did NOT use that. But of course, I couldn't get his code to work. – Kim Stacks Nov 28 '12 at 06:28
  • 1
    I have edited my answer to try to explain the reason for `defer`, as well as some other things. Cheers :) – Nathan Wall Nov 28 '12 at 06:49
  • Excellent answer, Nathan! You're the boss! – Kim Stacks Nov 28 '12 at 07:07
  • 1
    Thanks kimisia. I just updated the original code to use the promise returned by `getSizeSettingsFromPage` to call `calculateRatio` without the need to pass in the `whenReady` parameter. As long as you're using promises, you might as well go all the way :-). Promises eliminate the need for those kinds of callbacks. – Nathan Wall Nov 28 '12 at 07:26
  • Just to add that I went back to my original code and refactored it to include Q.js Promises code. And it worked well. Thank you. – Kim Stacks Nov 28 '12 at 08:11
  • Oh thank you, Nathan! Seriously, I do not have an intuitive understanding of callbacks, so Promises with the keywords "then" really made it easier for me to read the code. – Kim Stacks Nov 28 '12 at 08:17
  • Hi Nathan, I am currently facing a css/javascript issue at http://stackoverflow.com/q/13910422/80353 Was wondering if you can offer some advice. – Kim Stacks Dec 17 '12 at 09:01
  • Baller. An explanation like this should be in `Q`'s readme. Right now it seems to assume too much familiarity with the idea of promises. Maybe you could edit it and submit a pull request? `:D` – Will Jul 05 '13 at 19:05
  • Q.all([ addAsync(1, 1), addAsync(2, 2), addAsync(3, 3) ]).spread(function(result1, result2, result3) { console.log(result1, result2, result3); }); – clevertension Mar 08 '14 at 10:01
  • What about the failure function? Do you get all the failed promises, or the most recent one? – CMCDragonkai Mar 15 '14 at 22:25
2

Looks like you should use the Q.all function to create a master promise corresponding to when all the getSizeSettings promises are fufilled.

https://github.com/kriskowal/q#combination

var ps = [];
for (var i=0, l=pages.length; i<l; i++) {
   ps[i] = getSizeSettingsFromPage(pages[i], calculateRatio);   
}

Q.all(ps).then(function(){ callWhenReady() })

Most promise libraries should provide a similar method to do this kind of synchronization. If you ever come across one that does not what you could do is hook each individual promise to a callback that increments a shared counter when its called. When your counter reaches n you know that you already resolved all promises so you can have the incrementor callback call the "real" callback as well.

//If you did not have Q.all available
//Or had to code this without a promise library

var to_go = pages.length;
for (var i=0, l=pages.length; i<l; i++) {
   getSizeSettingsFromPage(pages[i], calculateRatio)
   .then(function(){
       to_go--;
       if(to_go == 0){
           callWhenReady()
       }
   });
}

Note that in these cases until now the async calls are allowed to run parallel. If you need them to run sequentially then usually the only way is to rewrite the for loop as a recursive function

var go = function(i){
    if(i>=pages.length){
        return call_next_step()
    }else{
        return do_ith_calculation(i)
        .then(function(){
            return go(i+1)
        })
    }
};
go(0);
hugomg
  • 68,213
  • 24
  • 160
  • 246
  • I tried https://github.com/simkimsia/learn-promises-javascript/commit/69db89b95bfd87437c30c76f763199dc53530fad which used your 2nd solution. The moment I drag and drop the files, the code broke. I cannot find anything wrong by looking at console. – Kim Stacks Nov 28 '12 at 06:12
  • i also tried the Q.all https://github.com/simkimsia/learn-promises-javascript/commit/5084edc25a68b378be1496292f645812776dd7ce the code can run successfully, but the result returned was still 0. – Kim Stacks Nov 28 '12 at 06:13
  • i did not try the recursive function, because i do not have an intuitive understanding of it. – Kim Stacks Nov 28 '12 at 06:13
  • thanks for answer, missingno. I figured it out with Nathan's answer! Cheers :) – Kim Stacks Nov 28 '12 at 07:09
  • @kimsia: the recursive loop thing is only if you want to *chain* the async stuff one after the other. The second solution is how Q.all gets implemented under the hood - you only need it if you don't have Q.all available and need to code something lik eit yourself. – hugomg Nov 28 '12 at 13:51