CODEWITHSUNDEEP
javascriptpromisenested
Hugo
Hugo

Reputation: 67

Is this type of promise nesting good practice?

I just wanted to know if it is considered good practice to nest promises like in this example, or is there better alternatives ?

getDatabaseModel.searchId(idNoms).then(function([noms, elements]) {
        getDatabaseModel.getLocalisations().then(function(localisations){
            getDatabaseModel.getStates().then(function(states){
                //Some code
            })
        })
    })

Upvotes: 2

Views: 103

Answers (3)

Nguyễn Văn Phong
Nguyễn Văn Phong

Reputation: 14208

Obviously, your promises are independent. So you should use Promise.all() to make it run parallel with the highest performance.

The Promise.all() method takes an iterable of promises as an input, and returns a single Promise that resolves to an array of the results of the input promises

var searchById = getDatabaseModel.searchId(idNoms);
var getLocalisations = getDatabaseModel.getLocalisations();
var getStates = getDatabaseModel.getStates();

var result = Promise.all([searchById, getLocalisations, getStates]);
result.then((values) => {
  console.log(values);
});

For example, Let's say each promise takes 1s - So it should be 3s in total, But with Promise.all, actually it just takes 1s in total.

var tick = Date.now();
const log = (v) => console.log(`${v} \n Elapsed: ${Date.now() - tick}`);

log("Staring... ");
var fetchData = (name, ms) => new Promise(resolve => setTimeout(() => resolve(name), ms));
var result = Promise.all(
            [
              fetchData("searchById", 1000), 
              fetchData("getLocalisations", 1000),
              fetchData("getStates", 1000)
            ]);
result.then((values) => {
  log("Complete...");
  console.log(values);
});

Besides, If you're concern about asyn/await with more elegant/concise/read it like sync code, then await keyword much says the code should wait until the async request is finished and then afterward it'll execute the next thing. While those promises are independent. So promise.all is better than in your case.

var tick = Date.now();
const log = (v) => console.log(`${v} \n Elapsed: ${Date.now() - tick}`);

var fetchData = (name, ms) => new Promise(resolve => setTimeout(() => resolve(name), ms));
Run();

async function Run(){
  log("Staring... ");
  
  var a = await fetchData("searchById", 1000);
  var b = await fetchData("getLocalisations", 1000);
  var c = await fetchData("getStates", 1000);
  
  log("Complete...");
  console.log([a, b, c]);
}

Upvotes: 3

Sam R.
Sam R.

Reputation: 16450

IMO it's a little hard to read and understand. Compare with this:

getDatabaseModel.searchId(idNoms)
    .then(([noms, elements]) => getDatabaseModel.getLocalisations())
    .then(localization => getDatabaseModel.getStates());

As @deceze pointed out there are two things to note:

  • These functions are called serially
  • They don't seem to depend on each other as the noms, elements and localization are not used at all.

With Promise.all you can mix and match however you want:


// Call `searchId` and `getState` at the same time
// Call `getLocalisations` after `searchId` is done
// wait for all to finish
Promise.all([
  getDatabaseModel.searchId(idNoms).then(([noms, elements]) => getDatabaseModel.getLocalisations()),
  getDatabaseModel.getStates()
]).then(([result1, result2]) => console.log('done'));


// Call all 3 at the same time
// wait for all to finish
Promise.all([
  getDatabaseModel.searchId(idNoms),
  getDatabaseModel.getLocalisations(),
  getDatabaseModel.getStates(),
]).then(([result1, result2, result3]) => console.log('done'));

Upvotes: 2

Sudhanshu Kumar
Sudhanshu Kumar

Reputation: 2044

Promises were made to avoid callback hell, but they are not too good at it too. People like promises until they find async/await. The exact same code can be re-written in async/await as

async getModel(idNoms){
  const [noms, elements] = await getDatabaseModel.searchId(idNoms);
  const localisations = await getDatabaseModel.getLocalisations();
  const state = await getDatabaseModel.getStates():
  // do something using localisations & state, it'll work

}

getModel(idNoms);

Learn async/await here

Upvotes: 3

Related Questions