How to loop in node js, wait until each iteration

Question

I have a function:

var third = function(classes){
  for (var i = 0; i <= (classes.length-1); i++) {
       var Myurl = classes[i];

       return function(Myurl){
          request(Myurl,  function(err, resp, body) { 
             if (!err && resp.statusCode == 200) {
                var $ = cheerio.load(body);
                $("#item_details dl").each(function() {
                   var Values = [];
                   var New = [];

                   Values=$(this).find("dd strong").text();

                   New = Values.replace(/[\n\t\r]/g,"");
                   AllLinks.push(New);
                });

                console.log(AllLinks);
             };
          })
       }(MyUrl);

  };
};

The problem is when I do the above I only get the result of first loop element (i=0) in the console.log(AllLinks). How do I properly loop in node? I am new to node so any comments are much appreciated!

EDIT: If I define AllLinks in request then it seems to work, but not in correct order...

var third = function(classes){
      for (var i = 0; i <= (classes.length-1); i++) {
           var Myurl = classes[i];

            (function(Myurl){
              request(Myurl,  function(err, resp, body) { 
                 if (!err && resp.statusCode == 200) {
                    var $ = cheerio.load(body);
                    AllLinks=[];
                    $("#item_details dl").each(function() {
                       var Values = [];
                       var New = [];

                       Values=$(this).find("dd strong").text();

                       New = Values.replace(/[\n\t\r]/g,"");
                       AllLinks.push(New);
                    });

                    console.log(AllLinks);
                 }(Myurl);
              })
           };

      };
    };

@CallumLinington Thanks, I am a node beginner, so you maybe can provide an example about how the above function should be changed? best regards — user1665355, May 26 '15 at 10:02
Why do you want to return call result return function(Myurl){...}(MyUrl);? In certain scenarios it might be enough. Otherwise you might want to try promises or async library — Igor Popov, May 26 '15 at 10:03
I'm not sure how you implement request but if you drop 'return' you'll most likely be able to call all your urls from classes. If you want to wait for all calls to succeed async.js sounds like your choice. — Igor Popov, May 26 '15 at 10:09
I think the solution is much simpler. Instead of return function in the loop, remove return and have it be only function — AmmarCSE, May 26 '15 at 10:09
@AmmarCSE they are initialized in the global environment, outside of the function. — user1665355, May 26 '15 at 10:21
@user1665355, ok, what results are given when you try my answer? — AmmarCSE, May 26 '15 at 10:23
@user1665355, they will not be in any gauranteed order because request() is asynchronous — AmmarCSE, May 26 '15 at 10:42

score 4 · Accepted Answer · edited May 23 '17 at 12:16

4

The major problem (apart from 'return') is that assuming request performs asyncronous operation your function returns when request is not completed and thus log contains no update.

You have two strategies in general:

call your code without 'return' as in the example above. This case all your requests will be eventually done but you have no control over when. It works well when you don't need them all to proceed, e.g. process AllLinks afterwards
Use any technology that supports waiting until all your calls are completed (async.js or promises). For example here Simplest way to wait some asynchronous tasks complete, in Javascript?

Thus you need:

function appendResultToItems(url, callback) {
 request(url,  function(err, resp, body) { 
   if (!err && resp.statusCode == 200) {
     var $ = cheerio.load(body);
     $("#item_details dl").each(function() {
       var Values = [];
       var New = [];

       Values=$(this).find("dd strong").text();

       New = Values.replace(/[\n\t\r]/g,"");
       AllLinks.push({result:New, url: url});
       callback();
     });
 });
}

var calls = [];

classes.forEach(function(Myurl){
  calls.push(function(callback) {
    appendResultToItems(Myurl, callback);
  });
});

async.parallel(calls, function() {
  console.log(AllLinks);
});

edited May 23 '17 at 12:16

Community

1
1

answered May 26 '15 at 10:36

Igor Popov

2,084
16
18

Great answer, please see my edit. Is that what you mean, the edit code works but AllLinks are not in correct order... – user1665355 May 26 '15 at 10:41
You can't have them in order this case. What you can do is to write results by url or classes index and then reorder them – Igor Popov May 26 '15 at 10:47
Btw, what is "callback" function in the example above? – user1665355 May 26 '15 at 19:50
It's the most simple way of notifying the function has completed. Normally it is called with result and/or error as parameters – Igor Popov May 26 '15 at 19:52
So the callback is asyncsCallback you mean? – user1665355 May 27 '15 at 13:51
I'm not sure I understand. Callback pattern is a default way of handling async calls in node. At the same moment it's just a function accepting some parameters according to convention and not of any specific type. For example in the last three lines of my example 'function' is a callback function. – Igor Popov May 27 '15 at 13:57
What I meant was, see the link http://www.sebastianseilund.com/nodejs-async-in-practice. In the comment field, a guy wrote: "One slightly nit-picky thing is the naming of everything callback. It took me a few minutes to grock that the callback being used in those spots was actually the async module's callback and not meant to be one of mine." – user1665355 May 27 '15 at 14:06
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/78919/discussion-between-igor-and-user1665355). – Igor Popov May 27 '15 at 14:18

score 1 · Answer 2 · answered May 26 '15 at 10:03

1

Use async.js#eachSeries to apply an asynchronous function to each element of a collection

answered May 26 '15 at 10:03

Sergey Grigoriev

709
7
15

thanks, but do you have an example of how the above function should be changed? I am a node beginner... – user1665355 May 26 '15 at 10:05
@user1665355 on async.js you will find more than examples – vijay May 26 '15 at 10:07

score 1 · Answer 3 · answered May 26 '15 at 10:13

Your problem is you are using return within the loop. If you simply use an IIFE

var third = function(classes){
  for (var i = 0; i <= (classes.length-1); i++) {
       var Myurl = classes[i];

       (function(Myurl){
          request(Myurl,  function(err, resp, body) { 
             if (!err && resp.statusCode == 200) {
                var $ = cheerio.load(body);
                $("#item_details dl").each(function() {
                   var Values = [];
                   var New = [];

                   Values=$(this).find("dd strong").text();

                   New = Values.replace(/[\n\t\r]/g,"");
                   AllLinks.push(New);
                });

                console.log(AllLinks);
             };
          })
       })(MyUrl);

  };
};

AllLinks will be correctly generated.

that does not give the correct results unfortunately! AllLinks are not correct... — user1665355, May 26 '15 at 10:16

How to loop in node js, wait until each iteration

3 Answers3