Nested For Loops converted to Nested Promises

2019-05-11 06:29发布

问题:

I'm running into a problem where my program ends only on one iteration of the nameList, and I'm not exactly sure where the illogical code lies.

Given globally:

var _ = require("underscore");
var nameList = ["Bob", "Susie"]
var jsonDict = {}

My complicated nesting starts here, but I'm not sure how to fix it so that it iterates through both the nameList and the numbers for-loop 1-10:

return new Promise((res, rej) => {
    var promises = [];
    return Promise.map(nameList, function(personName){
        for (var j=1; j<=10; j++){
            return promises.push(new Promise(function(resolve, reject) {
                params['page'] = j;

                console.log('PAGE ' + j + ' ' + personName)

                SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params).then(function(data){
                    return Promise.map(data, function(item, index){
                        if (!_.has(jsonDict, item.key)){
                            jsonDict[item.key] = {
                                name: personName
                            }
                        }
                        return
                    }).then(function(){
                        console.log(jsonDict)
                        return resolve(true)
                    })

                }).catch(function(err){
                    console.log(err)
                    return reject(false)
                })
            }))
        }
    }).then(function(){
        return res(true)
    })
}).then(function(){
    console.log('Done with everything.')
})

I get the following output:

PAGE 1 Bob
PAGE 1 Susie
Done with everything.

{ 
    '12345': { name: "Bob" },
    '12346': { name: "Bob" },
    ...
    '99999': { name: "Bob" }
}

and I never get the data for Susie, I'm returning early but can't seem to figure out where. Any help/guidance in where the problem lies (or even refactoring) would be appreciated. Thanks in advance!

回答1:

First: Your code is fundamentally flawed

Why I say fundamentally flawed is because you seem to misunderstand how functions work

return Promise.map(nameList, function(personName){  <-- this function 
  for (var j=1; j<=10; j++){
    return promises.push(new Promise(function(resolve, reject) { <-- is returning HERE

Here Array.prototype.push functionreturns the index of the newly add item. (I looked it up,TIL)
The biggest issue here is that you are RETURNING it.

As soon as you return the mapper function of Promise.map you're telling it that your done! So in your code above, it wouldn't even get to the next iteration of the for loop

Look up specifications in MDN for return its says

The return statement ends function execution and specifies a value to be returned to the function caller.

Hopefully this answer your question to where you are returning early.


Second: I wouldn't count on your Promises

I too had myself a difficult time grokking promises. Let me direct you to an awesome articles on promises I recently read. Now let me apply what I learned from that. Thank you for this question.

Promises we're made to combat callback hell but you skewered all that it was made for.

I'll try to flatten the promises starting from the most nested piece

SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params).then(function(data){
    return Promise.map(data, function(item, index){
        if (!_.has(jsonDict, item.key)){
            jsonDict[item.key] = {
                name: personName
            }
        }
        return
    }).then(function(){
        console.log(jsonDict)
        return resolve(true)
    })

}).catch(function(err){
    console.log(err)
    return reject(false)
})

The inner promise map here is unnecessary, you could just use a standard map for even a for loop since you're not actually mapping anything here...

[Refactor 1]

// Create subroutine, careful with the globals...
let populateJsonDict = singleData => {
  if (!_.has(jsonDict, singleData.key)) jsonDict[singleData.key] = { name: personName }
}

SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params).then(data => {
    data.forEach(populateJsonDict);
    resolve(true); // this will be removed later
}).catch(function(err){
    console.log(err);
    reject(false); // this will be removed later
})

Okay let's move up the callback pyramid (or move down?). Next on the list

for (var j=1; j<=10; j++){
  return promises.push(new Promise(function(resolve, reject) {
    params['page'] = j;

    console.log('PAGE ' + j + ' ' + personName)

    //[Refactored 1]
  }))
}

Again another unnecessary Promise here, time to get rid of that. And that cornerstone of the issue here, the return

[Refactor 2]

for (var j=1; j<=10; j++){
  //[from Refactored 1]
  let populateJsonDict = singleData => {
    if (!_.has(jsonDict, singleData.key)) jsonDict[singleData.key] = { name: personName }
  }
  params['page'] = j; // I assume this a global somewhere
  let p = SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params).then(data => {
    data.forEach(populateJsonDict);
    // Removed because we've removed the parent promise
  }).catch(function(err){
    console.log(err);
    // Removed because we've removed the parent promise
  })
  promises.push(p)
}

I'll fastforward as i noticed this is getting really long. Next piece to refactor

return new Promise((res, rej) => {
    var promises = [];
    return Promise.map(nameList, function(personName){
      //[Refactored 2]
    }).then(function(){
      return res(true)
    })
}).then(function(){
    console.log('Done with everything.')
})

I don't really know how to salvage this so I'll write things from group up.

[Refactor 3:final]

var promises = [];
nameList.forEach(personName => { // Like earlier, redundant Promise.map
  //[from Refactored 2]
  for (var j=1; j<=10; j++){
    let populateJsonDict = singleData => {
        if (!_.has(jsonDict, singleData.key)) jsonDict[singleData.key] = { name: personName }
    }
    params['page'] = j;
    let p = SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params).then(data => {
        data.forEach(populateJsonDict);
    }).catch(function(err){
        console.log(err);
    })
    promises.push(p)
  }
});
// At this point you have an array of Promises, for this we can utilize Promise.all

Promise.all(promises)
    .then(() => console.log('Done with Everything'));

I suppose that could have been done better. Let me do one last version of this.

[Refactor 3.1:final]

let populateJsonDict = name => key => !_.has(jsonDict, key) && Object.assign(jsonDict, {[key]:name};
let promises = _.times(10, 
    index => {
        params['page'] = index+1;
        return Promise.map(nameList, name => {
            let populateWithName = populateJsonDict(name);
            let iterate = data => data.forEach(populateWithName);
            return SOME_SEARCH_FX_THAT_RETURNS_A_PROMISE(params)
                .then(iterate)
                .catch(err => console.log(err));
        });
    });

Promise.all(promises)
    .then(() => console.log('Everything done'));

Okay, still have this felling of un-satisfaction but that's what I have for now... That was really more for me than you. So thank you again friend. I hope we both can continue to advance is this ever changing field of work. Apologies if all this sounds condescending in anyway. Cheers!