Node.js - Async Whilst loop not iterating

Question

Based off suggestions from a previous question Illegal break statement (Node.js) , I implemented async.whilst(), but it is not iterating more than once.

I am trying to find a unique ID, by incrementing a number at the end of an ID, and querying Mongo to see if that ID exists. If it doesn't exist, the unique ID is found. It is only looping once, instead of until a unique is found. What is wrong?

The code:

 var uniqueNumber = 1;
 var newUnique;

 async.whilst(
    function () { 

       var uniqueNum_string = uniqueNumber.toString(); 
       newUnique = data.id + uniqueNum_string;

       db.collection('landmarks').findOne({'id':newUnique}, function(err, data){

           if (data){
              console.log('entry found!');
              return;
           }

           else {
              console.log('entry not found!');

           }
        });

  },
  function (callback) {

     uniqueNumber++;

   },
   function (err) {

      saveLandmark(newUnique);
   }
);

Answer 1

I couldn't actually find a good async function suited for this task, so I hacked something together using async.forever(). That function will keep running until you callback an "error", which is what you want to do.

var uniqueNumber = 1;
var newUnique;

async.forever(function (next) {
  var uniqueNum_string = uniqueNumber.toString(); 
  newUnique = data.id + uniqueNum_string;

  db.collection('landmarks').findOne({'id':newUnique}, function(err, data){
    if (data){
      console.log('entry found!');
      uniqueNumber++;
      next();
    }

    else {
      console.log('entry not found!');
      next('unique!'); // This is where the looping is stopped
    }
  });
},
function () {
  saveLandmark(newUnique);
});

Regarding what you're trying to solve, it seems to me you want to insert a new document with a unique id. If that is the case and you're gonna do it often, I would say this is a highly inefficient approach. If you have a thousand documents in the database you would do a thousand completely meaningless requests to the database before you even get close to a unique id.

A better approach would be to get the first document from the collection sorted descending by id (eg the highest id). Then increase that id by one and try to insert until it's not rejected. Because even if you find a unique id, by the time you save the document another insert might have been made from another client or another instance (in case of load-balancing). This may or might not be a problem in your case, I don't know enough about your app, I just thought you should be aware of the drawbacks with your current approach and my answer.

Answer 2

SOmething like this? I didn't test it because I don't know what DB module you're using, but the logic should be obvious.

function searchNubmersForResults(firstNumber, callback) { 
    //place other variables here, and you can collect the results within the closure, and send them as arguments to your callback
   function testNumber(uniqueNumber) {
        var uniqueNum_string = uniqueNumber.toString(); 
        newUnique = data.id + uniqueNum_string;

        db.collection('landmarks').findOne({'id':newUnique}, function(err, data){

            if (data){
                console.log('entry found!');
                callback(data);//We're done, use the callback on the data
            } else {
                console.log('entry not found!');
                testNumber(uniqueNumber++);//Launch the next test
            }
        });
    }

    testNumber(firstNumber);//Laucn the first test
}

searchNubmersForResults(0, function(data) {
    console.log('You have data now: ' + data);
});