up vote 0 down vote favorite

I wonder what is the proper way to do this?

while(newList.length < 10){
            async.series([
                function(callback){         
                    async.eachSeries(rest,function(id,next){
                        if(getDistance(myLoc,id) < 5){
                            newList.push(id);
                        }
                        next();
                    });
                    callback();
                },
                function(callback){
                    var size = 10 - newList.length
                    rest = getPlaces()
                    callback();
                }], function(err,results){
                    if(err) console.log('error occured in : ' + err);
                });
        }

rest is populated by 10 new places every run, while the first step of the asyc process is to find out if the distance of the 10 places is within 5km, if its within 5 km, then it's added to newList, if not then we'll do another loop until it finds 10 places with its distance within 5km

while really the idea is to re-iterate until my newList is filled with 10 (places with distance < 10 km), when I try to run this, it went over everything just once and won't loop for the second run.

share|improve this question
1  
your code is pretty complicated, and most of the users won't even try to understand it, can you simplify it by removing needless parts? – Hatsuney Sep 28 '16 at 12:44
    
Done, I hope it's simple enough @Hatsuney – kenlz Sep 28 '16 at 12:50
up vote 1 down vote accepted

Okay, so you have several problems here.
Let's start with this:

function(callback){         
    async.eachSeries(rest,function(id,next){
        if(getDistance(myLoc,id) < 5){
            newList.push(id);
        }
        next();
    });
    callback();
}

You're not waiting for the output of eachSeries, you're starting it, and immediately calling back.
That function will fill out your newList independently, maybe immediately, maybe after a slight delay; you can't know that, because you called back right away.
You also don't gain anything by using series here, because it's supposed to run functions each one after the previous function has completed.
Your code might end up working like this:

  • start async.series
  • run the first function
  • the function begins an async.eachSeries
  • the function calls back
  • second function starts, and newList is still not filled
  • second function ends
  • end of the loop
  • newList is still not filled

Other than that, your while function doesn't have to wait for anything either, it will just mindlessly run async.series over and over again, until your newList gets filled, most likely taking up tons of resources. You can use functions like async.whilst instead, for example:

async.whilst(
    function() { return newList.length < 10; },
    function(callback) {
        ...
    }, function(err, results) {
        console.log('loop has ended')
    }
)

Also, you should probably use the callbacks from series to generate a results array, and process them at the end of a loop, instead of letting the functions fill stuff out randomly and unpredictably.
For example:

function(callback){
    var list = []
    async.eachSeries(rest,function(id,next){
        if(getDistance(myLoc,id) < 5){
            list.push(id);
        }
        next();
    }, function(err) {
        callback(err, list);
    });
}

I hope this shed some light on your problems.

share|improve this answer
    
I tried this way, but ended up the list filling up to 4xx, while it should end the loop when the list is 10. I wonder what happened? – kenlz Sep 29 '16 at 7:39
    
take it slow, step by step, making sure everything works as intended on the way – Hatsuney Sep 29 '16 at 17:19
    
Nvm got it! I forgot to call the callback. Thanks! – kenlz Sep 30 '16 at 0:41
up vote 1 down vote

Use async whilst instead of a regular while:

var newList = [];
async.whilst(
    function(){ return newList.length < 10;},
    function(done) {
        async.series([
           function(callback){  
               var size = 10 - newList.length
               rest = getPlaces()
               callback();       
           },
           function(callback){
               async.eachSeries(rest,function(id,next){
                   if(getDistance(myLoc,id) < 5){
                        newList.push(id);
                   }
                   next();
               });
               callback();
           }], 
           function(err,results){
               if(err) console.log('error occured in : ' + err);
               done(err, newList);
           };    
    },
    function(results) {
        // New list has been filled
    }
})
share|improve this answer
up vote 0 down vote

My suggestion would be to have a boolean variable outside the while scope, which is by default true, and which is set to false when the async function fills the list with 10 elements. Hope that helps.

share|improve this answer
    
Also, before actually using the list variable in other context, you should check whether, the filling condition is full, as JS will continue running async code in the background and may start a function on the yet-unpopulated variable from memory instead the instance of the variable, that is being populated by the async code. – Erikzy Sep 28 '16 at 12:58

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.