current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite

I haven't really worked much with asynchronous code so I'm wondering if I am doing something wrong/something could go wrong. Right now of all the test cases that I have done, my code has worked but I'm not sure if I will get side effects or not.

I won't post any of the business logic, just the iteration part with the asynchronous function in it:

function asyncInIteration(skinArray)  {
   request("/someaddresshere", function (err, response, data)  {

      var left = something.length;

      for (var i = 0;i<something.length;i++) (function (i) {
          request("/someotheraddress", function (err, response, data) {
             if (--left === 0)  {
                 console.log("End of for loop");
             }
          })
      })(i);
   });
}

Basically, what I am trying to do with this code is loop through an array (skinArray) and add values to it via sending HTTP requests.

The most important thing for me is that:

  1. Every value of i should only be repeated once.
  2. When I call --left === 0, that has to be called once, and only after all values of i have been gone through.

If someone wants to see the code with the business logic, you can find it here.

share|improve this question
    
This will properly detect when the last request() operation finishes. Do you need the request() results in order when you're done? Are you OK with all the request() operations being launch in parallel (all in-flight at the same time)? – jfriend00 Mar 9 at 5:54
    
Yeah I don't mind if they aren't in order. The only thing that matters for me is that everything in the array only iterates once and the --left === 0 conditional is at the end of the for loop. – mre12345 Mar 9 at 7:48
    
Your wording makes me wonder if you understand the actual sequence of events here. First, your for loop runs and initiates all the requests. At that point, the for loop is done and any code that comes after it executes. Then, some time later the first request() finishes and calls its callback. Then, sometime later, the next does. Then, finally the last one completes and your left variable will hit 0. The for loop was done long before this. – jfriend00 Mar 9 at 7:50
    
Thanks for this! – mre12345 Mar 10 at 5:40
up vote 1 down vote

Use lodash to make Your iteration "readable".

And use async to handle end of asynchronous iteration to properly respond with "end of loop"

var _ = require('lodash');

var something = ['a','b','c'];
function asyncInIteration(skinArray)  {

   request("/someaddresshere", function (err, response, data)  {
      var functions = []; // array of functions that will be called

      // generating array of functions
      _.times(something.length, 
          function() {
              // pushing function to functions list
              functions.push(function(done){ // "next" is necessary to inform async module about finishing call
                  request("/someotheraddress", function (err, response, data) {
                      // doing something
                      console.log("\n--------");
                      console.log("RESPONSE:", data);
                      console.log("--------\n");
                      done();
                  });
              });
          });

      // can change to .series if You want to make it synchronous (not parallel)
      async.parallel(functions, function(){
          console.log('End of loop');
      });
   });
}

P.S. In fact my code looks like not so simple, but there is no another elegant way to control asynchronousity. Maybe Promise object will be easy to work, but I don't think it's suitable for current task.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.