2

This is my simple task: Find images by id array and render images value into template.

router.get('/gallery', function(req, res) {
  var images = [];
  imagesIds.forEach(function(eachImageId) {
    Images.findById(eachImageId).exec(function(findImageErr, foundImage) {
      if (foundImage) {
        images.push(foundImage);
      }
    });
  });
  res.render('gallery', {
    images: images
  });
});

The problem is the 'res.render' function does not wait for 'findById' function to finish. 'images' array always become '[]' empty.

I try to use generator but did not know how to achieve.

If someone can explain without library(like q) will be better. Because I want to know generator deeply how to deal with this problem.

thefourtheye
  • 233,700
  • 52
  • 457
  • 497
JamesYin
  • 732
  • 9
  • 19
  • Like this: http://recurial.com/programming/understanding-callback-functions-in-javascript/? %)P Make an own callback. – loveNoHate Sep 07 '15 at 03:56
  • As you want to render an image when image is available, but, image is available in callback function. So put your `render()` in callback function. – Gaurav Gupta Sep 07 '15 at 05:30
  • @GauravGupta Thanks. Pelit Mamani just make a solution as your description. – JamesYin Sep 07 '15 at 05:53
  • No, generators don't deal with this problem. I'm not sure why you think they'd do? Promises will help you here. – Bergi Sep 07 '15 at 13:46
  • I assume `imagesIds` is an `Array`, not something custom with a `forEach` method? – Bergi Sep 07 '15 at 13:47
  • @Bergi Many articles compare `callback` , `generator` and `promises` in asynchronous functions. So I think generator is a one of the way, I do not know which one is better. Yes, imagesIds is an `Array`. – JamesYin Sep 08 '15 at 07:06
  • @JamesYin: Generators only provide syntactic sugar for promises, as they can be used to polyfill `async/await`. Try with plain promises first. – Bergi Sep 08 '15 at 10:03
  • @Bergi I haven't notice. Thanks your hint. I will try it. – JamesYin Sep 09 '15 at 01:16

3 Answers3

2

Generators allow to write synchronous-like function, because they can stop its execution and resume it later.

I guess you already read some articles like this and know how to define generator function and use them.

Your asynchronous code can be represented as a simple iterator with a magic yield keyword. Generator function will run and stop here until you resume it using method next().

function* loadImages(imagesIds) {
    var images = [], image;
    for(imageId of imagesIds) {
        image = yield loadSingleImage(imageId);
        images.push(image);
    }
    return images;
}

Because there is a cycle, function will go though the cycle with each next() until all imagesIds will have been walked. Finally there will be executed return statement and you will get images.

Now we need to describe image loading. Our generator function need to know when current image have loaded and it can start to load next. All modern javascript runtimes (node.js and latest browsers) have native Promise object support and we will define a function which returns a promise and it will be eventually resolved with image if it will have been found.

function loadSingleImage(imageId) {
    return new Promise((resolve, reject) => {
        Images.findById(imageId).exec((findImageErr, foundImage) => {
            if (foundImage) {
               resolve(foundImage)
            } else {
               reject();
            }
        });
    });
}

Well we have two functions, one for single image load and the second for putting them together. Now we need a some dispatcher for passing control from one to another function. Since your don't want to use libraries, we have to implement some helper by yourself.

It is a smaller version of spawn function, which can be simpler and better to understand, since we don't need to handle errors, but just ignore missing images.

function spawn(generator) {
    function continuer(value) {
         var result = generator.next(value);
         if(!result.done) {
             return Promise.resolve(result.value).then(continuer);
         } else {
             return result.value;                 
         }
    }
    return continuer();
}

This functions performs a recursive calls of our generator within continuer function while the result.done is not true. Once it got, that means that generation has been successfully finished and we can return our value.

And finally, putting all together, you will get the following code for gallery loading.

router.get('/gallery', function(req, res) {
    var imageGenerator = loadImages(imagesIds);
    spawn(imageGenerator).then(function(images) {
        res.render('gallery', {
            images: images
        });
    });
});

Now you have a little bit pseudo-synchronous code in the loadImages function. And I hope it helps to understand how generators work.

Also note that all images will be loaded sequently, because we wait asynchronous result of loadSingleImage call to put it in array, before we can go to the next imageId. It can cause performance issues, if you are going to use this way in production.

Related links:

just-boris
  • 9,468
  • 5
  • 48
  • 84
  • You have no idea how grateful I am. I can basically understand how to use `generator` and `promises` to solve my asynchronous functions problems. Just need some time to digest some detail. One more question: If `promise` can deferred a function why we use `yield` to pause `loadSingleImage(imageId);` function? – JamesYin Sep 08 '15 at 08:30
  • In this example, yield is used just because to show how it works. You could get same result using `Promise.all(imagesIds.map(loadSingleImage))` – just-boris Sep 08 '15 at 09:15
1

It can be done without a 3rd party as you asked, but it would be cumbersome... Anyway the bottom line is to do it inside the callback function "function(findImageErr,foundImage){..}".

1) Without a 3rd party you - you need to render only after all images were accounted for:

 var images = [];
  var results=0;
  imagesIds.forEach(function(eachImageId) {
    Images.findById(eachImageId).exec(function(findImageErr, foundImage) {
        results++;
        if(foundImage) 
              images.push(foundImage);
        if(results == imagesIds.length)
              res.render('gallery',{images:images});
    });
  });    

2) I strongly recommend a 3rd party which would do the same. I'm currently using async, but I might migrate to promises in the future.

async.map(
    imageIds,
    function(eachImageId,next){
         Images.findById(eachImageId).exec(function(findImageErr, foundImage) {
                next(null,foundImage);
                // don't report errors to async, because it will abort
        )
    },
    function(err, images){
          images=_.compact(images); // remove null images, i'm using lodash
          res.render('gallery',{images:images});
    }
);

Edited: following your readability remark, please note if you create some wrapper function for 'findById(...).exec(...)' that ignores errors and just reports them as null (call it 'findIgnoreError'(imageId, callback)) then you could write:

async.map(
   imageIds,
   findIgnoreError,
   function(err, images){
              images=_.compact(images); // remove null images, i'm using lodash
              res.render('gallery',{images:images});
   }
);

In other words, it becomes a bit more readable if the reader starts to think Functions... It says "go over those imageIds in parallel, run "findIgnoreError" on each imageId, and the final section says what to do with the accumulated results...

Pelit Mamani
  • 2,321
  • 2
  • 13
  • 11
  • Yes, I know first solution. As you said, it is not elegant. So I pick another way. Second solution is not easy to read. But it works. – JamesYin Sep 07 '15 at 05:40
  • Yep, this "not easy to read" is a well known issue with asynchronous calls - consider how complex these instructions are: launch several actions in parallel, wait for results from all of them, and don't forget to handle errors... There were several well-known attempts to tackle this, promises & async are the most popular AFAIK, and I found it very useful (even if not perfect... e.g. I could use timeouts, and I could use a flag such as "ignore errors" that would save me the _.compact patch). – Pelit Mamani Sep 07 '15 at 09:16
  • Maybe I will use one of them if I go deeper in nodejs. But now I really confuse with AFAIK. Even after reading ton of documents I still can not get rip of my 'PHP way'. I really want to drill my brain and fill some document into it. – JamesYin Sep 07 '15 at 09:29
  • That's the only way to go :) If you ever feel like dropping a comment and sharing your insights it would be welcome . Also I edited my answer with a slight readability improvement, because IMHO one of the things to "wrap the brains about" is function objects (a bit like the "leap" a C programmer would do when learning to use function pointers).... cheers & good luck with the research. – Pelit Mamani Sep 07 '15 at 09:42
  • @JamesYin: FWIW, the second solution is implemented using the first solution. Only that you hide the `results` counter inside the `async.map()` function. That is the beauty of functional programming - you can make inelegant things look elegant all on your own without needing to wait for the language to grow new syntax. – slebetman Sep 08 '15 at 06:27
  • Here's a very old answer illustrating how functions from the `async` library can be implemented. As you can see, it's the same as the first solution: http://stackoverflow.com/questions/4631774/coordinating-parallel-execution-in-node-js/4631909#4631909 – slebetman Sep 08 '15 at 06:28
  • @slebetman Your link is so helpful. The chosen answer make a cleaver solution to deal with parallel processes. It opens my mind to go further in nodejs way. Thank you! – JamesYin Sep 08 '15 at 09:07
  • @slebetman After reading your link page. I realize the second solution using the same way as first one. – JamesYin Sep 08 '15 at 09:10
0

Instead of querying mongo(or any DB) N times, I would just fire a single query using $in:

Images.find({ _id : { $in : imagesIds}},function(err,images){
   if(err) return next(err);
   res.render('gallery',{images:images});
});

This would also reduce the number of io's, plus you won't have to write additional code to handle res.render

Vineet
  • 287
  • 2
  • 14
  • Thanks for your hint. Such a great solution. But I still confuse with asynchronous functions. Anyway I will keep reading. – JamesYin Sep 07 '15 at 05:50
  • What is that `cb` you're calling in case of an error? Shouldn't you rather render the error? – Bergi Sep 07 '15 at 13:48