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!
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!