Node.js call async function from within do while loop

Question

I want to call an async function from within a do while loop and exit only if the function doesn't return a value. Is there a way to manage this without having to actually wait outside the callback?

Here's the code that obviously always exits in the first loop since the function hasn't called back yet, when the while () is evaluated:

var i = 0;
do {
  var foundListing;
  if (i) {
    listing.slug = listing.slug + '-' + (i + 1);
  }
  listingService.getListingBySlug(listing, function(error, pFoundListing) {
    if (error) {
      return callback(util.errorParser(error));
    } else if (Object.keys(pFoundListing).length) {
      foundListing = pFoundListing;
      i++;
    }
  });
  //Do I have to wait here???
} while (foundListing && Object.keys(foundListing).length);

For Clarification: The point here is to generate a unique slug. If the slug already exists, i append a number and check again if it exists. When I get to the number that doesn't exist yet, I'm done.

Update: I found a way using recursion. I posted the working snippet as an answer.

You're trying to fit an asynchronous call in a synchronous structure. Basically the code continuously fire calls to listingService.getListingBySlug at each iteration of the loop, at maximum speed javascript can execute that loop. Are you sure is what you want to do? — Giacomo Cosimato
– Giacomo Cosimato, Commented Jan 17, 2017 at 9:48
@GiacomoCosimato It's the opposite. Since foundListing is empty in the beginning the while() part always evaluates to false in the first loop because it is set within the callback, which hasn't been executed yet. Either way, that's not what I want. I want to only enter the loop again, if the a listing has been found in listingService.getListingBySlug — Chry007
– Chry007, Commented Jan 17, 2017 at 9:56

Giacomo Cosimato · Accepted Answer · 2017-01-17 10:54:36Z

1

I don't have the possiblity to test it, but maybe a recursive function should do:

const listingService = {
  getListingBySlug(listing, callback) {
    setTimeout(() => {
      if (listing.slug === 'listing-5') {
        callback(
          false,
          {
            name: 'Listing 1',
            slug: 'listing-1',
          }
        );
      }
      else {
        callback(false, {});
      }
    }, 1000);
  },
};

function checkListingExists(slug) {
  return new Promise((resolve, reject) => {
		const listing = { slug };
    listingService.getListingBySlug(listing, (error, pFoundListing) => {
      if (error) {
        reject(error);
      } else if (Object.keys(pFoundListing).length) {
        resolve(pFoundListing);
      } else {
        resolve(false);
      }
    });
  });
}

function nextListing(prefix, index) {
  checkListingExists(prefix + '-' + index)
  .then((listing) => {
    if (listing === false) {
      nextListing(prefix, index + 1);
    } else {
      console.log(listing);
    }
  })
  .catch(() => {
    // deal with error response
  });
}

nextListing('listing', 0);

answered Jan 17, 2017 at 10:54

Giacomo Cosimato

6355 silver badges9 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Chry007 Over a year ago

Hey! Ty for the answer! I was just working on a recursive way to solve this. I also believe this is the only way. I will test and update.

Giacomo Cosimato Over a year ago

Consider that the approach is to loop server requests until you find a "non used slug", but this is far from optimal performance wise. I don't know if you own the API, but if you do, it would much better to implement a method like getFirstAvailableListing(slug) or something similar

Chry007 Over a year ago

I do own the API but the loop has to be somewhere on the server, doesn't it? The only other way I can think of is to fetch all listings with slug = newSlug* from the database and then determine via regex which would be the first non used slug

Giacomo Cosimato Over a year ago

That's right. The thing is, image if you have a thousand listings and you have to check each one of them in sequence, waiting every time for server response. If is the only option, than go for it of course.

Chry007 Over a year ago

The consideration is, that this should be an edge case. Users are allowed to create listings. The listing's title, that the user creates is turned into a slug. If the two users create listings with titles that generate the same slug this logic comes to live. I only check if the slug exists, if it does I append a number and check if that slug exists, etc.. It's highly unlikely that this has to be done more than a few times, even with 10k listings +. Good point to consider, still.

|

Piyush Beli · Accepted Answer · 2017-01-17 09:48:03Z

0

You should use promise in such cases. You can do something like this.

    var DEFERRED = require('deferred');
    var deferred = new DEFERRED();
    var i =0;

    while(1) {      
      listingService.getListingBySlug(listing, function(error, pFoundListing) {
    if (error) {
      return callback(util.errorParser(error));
      deferred.reject();
      break;
    } else if (Object.keys(pFoundListing).length) {         
      i++;
      listing.slug = listing.slug + '-' + (i + 1); 
      if(pFoundListing) {deferred.resolve()}
      break;
    }
  });


  deferred.promise.then(function() {
    // You will have the listing.slug value here.
  });

Btw you should not use Object.keys() to determine whether the object is empty or not. Simply create your own isEmpty method somewhere in utils file and check for the properties. If your pFoundListing is very large it will have the severe performance issue. For small objects (array) you would not notice the difference.

answered Jan 17, 2017 at 9:48

Piyush Beli

1932 silver badges8 bronze badges

2 Comments

Chry007 Over a year ago

Ty for the answer! But wouldn't that potentially loop 1.000 times if the callback takes long enough?

Soubhik Mondal Over a year ago

SyntaxError: Illegal break statement

Chry007 · Accepted Answer · 2017-01-17 11:19:28Z

I made it work using recursion as suggested by Giacomo Cosimato.

Here's the snippet that worked:

listingController.js

saveListing: function(listing, callback) {
  listingService.findOpenSlug(listing, listing.slug, 1, function(error, listing) {
    if (error) {
      return callback(util.errorParser(error));
    } else {
      listingService.saveListing(listing, function(error, savedListing) {
        //do some work
      });
    }
  });
}

listingService.js

var classMethods = {
  findOpenSlug: function(listing, baseSlug, i, callback) {
    listingRepository.getListingBySlug(listing.slug, function(error, foundListing) {
      if (error) {
        return callback(error);
      } else if (Object.keys(foundListing).length) {
        listing.slug = baseSlug + '-' + (i + 1)
        i++;
        classMethods.findOpenSlug(listing, baseSlug, i, callback);
      } else {
        callback(null, listing);
      }
    });
  },
  [...]
}

Collectives™ on Stack Overflow

Node.js call async function from within do while loop

3 Answers 3

6 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related