Broulaye Doumbia
Broulaye Doumbia

Reputation: 51

How to use loops in promises

I'm trying to execute a for loop within a promise with no success, i think my issue has to do with where to call resolve but i'm not sure

/*
* Get conversations of user
* @param user {String}
*/
function getConversations(user){
	return new Promise(function(resolve, reject){
		var conversations = user.Conversations
		var newConversations = []
		for(var conversation of conversations) {
			helperGetConvo(conversation.ConversID).then(function(convo){
				newConversations.push(createConversationObject({messages:[], name:convo.conversationName, users:["broulaye", "doumbia"], Id:convo.conversationID}))


			}).catch(function(reason) {
				console.log("failure when finding conversation 2: " + reason)
			})

		}
		resolve(newConversations)



	})
}


function helperGetConvo(convoId) {
	return new Promise (function(resolve, reject){
		query.findConversation(convoId).then(function(convers) {

			if(convers) {
				console.log("conversation was found: " + convers)
			}
			else {
				console.log("conversation was not found: " + convers)
			}
			resolve(convers)
		}).catch(function(reason) {
			console.log("failure when finding conversation: " + reason)
		})


	})
}

when I execute my code like this the getConversations function only returns an empty array. but when i change the getConversations function like so:

function getConversations(user){
	return new Promise(function(resolve, reject){
		var conversations = user.Conversations
		var newConversations = []
		for(var conversation of conversations) {
			helperGetConvo(conversation.ConversID).then(function(convo){
				newConversations.push(createConversationObject({messages:[], name:convo.conversationName, users:["broulaye", "doumbia"], Id:convo.conversationID}))
				resolve(newConversations)

			}).catch(function(reason) {
				console.log("failure when finding conversation 2: " + reason)
			})

		}
	})
}

i do get an output, however it does not get through the whole forloop i believe because from my understanding resolve work like a return statement.

someone help plz

Upvotes: 0

Views: 134

Answers (4)

TheF1rstPancake
TheF1rstPancake

Reputation: 2378

The problem is that when you call resolve you are resolving the entire promise. The for loop does not wait for each helperGetConvo() call to finish before moving on to the next one. Whichever of those promises hits the then statement first will call resolve and that's what your outer promise will resolve to.

You can read more on promises at: Understanding promises in node.js.

If you want to wait for a group of promises to finish, use Promise.all. It takes in a list of promises, and will only resolve if all promises complete successfully.

function getConversations(user) {
  return new Promise(function (resolve, reject) {
    var conversations = user.Conversations;
    var newConversations = [];
    //create a list of promises
    var promises = [];
    for (var conversation of conversations) {
      // push each promise into our array
      promises.push(
        helperGetConvo(conversation.ConversID).then(function (convo) {
          newConversations.push(createConversationObject({
            messages: [],
            name: convo.conversationName,
            users: ['broulaye', 'doumbia'],
            Id: convo.conversationID
          }));
        }).catch(function (reason) {
          console.log('failure when finding conversation 2: ' + reason);
        })
      );

    }
    // wait for all promises to complete
    // when then do, resolve the newConversations variable
    // which will now have all of the conversation objects that we wanted to create
    Promise.all(promises).then(() => resolve(newConversations)).catch(reject);
  });
}

You could also use async/await to clean this up. Async/await provides some nice syntactic sugar for removing the need to do return new Promise(...). This next code snippet is not the best way to use async/await because the for loop will process everything synchronously (one conversation at a time). This blog post has been hugely helpful for my understanding of using async/await in iterative problems: https://blog.lavrton.com/javascript-loops-how-to-handle-async-await-6252dd3c795.

async function getConversations(user) {
    var conversations = user.Conversations;
    var newConversations = [];

    // process each converstaion in sequence
    for (var conversation of conversations) {
      // instead of doing .then() we can use await
      // convo will have the result from the helperGetConvo
      // we put it in a try/catch because  output
      // we still want to have the error if something fails
      try {
        var convo = await helperGetConvo(conversation.ConversID);
        newConversations.push(createConversationObject({
          messages: [],
          name: convo.conversationName,
          users: ['broulaye', 'doumbia'],
          Id: convo.conversationID
        }));
      } catch(reason) {
        console.log('failure when finding conversation 2: ' + reason);
      }
    }

  // return
  return newConversations;
}

Async functions return promises. So you can call this function by doing getConversations(user).then(...). But I think async/await makes your code look much cleaner. There are definitely further optimizations that you can do, but hopefully this gets you started.

Upvotes: 1

Faz
Faz

Reputation: 379

You can loop a promise within a helper function that I found when I was trying to address a similar issue. I use this method to loop promises as it will not fall over at the first rejected promise. Instead I can handle the resolve or reject and return the final result once the loop has finished. The Promise in the code snippet below is using bluebird, http://bluebirdjs.com/docs/getting-started.html

    function promiseWhile(condition, action) {
        return new Promise((resolve, reject) => {

            var loop = () => {
                if (!condition()) return resolve();
                return Promise.cast(action())
                    .then(loop)
                    .catch(reject);
            };

            process.nextTick(loop);
            return resolve;
        })
    }

I modified your code examples provided with some dummy data and got it to work with the helper function. As a result I believe your getConversations function would look like this:

    function getConversations(user) {
        var conversations = user.Conversations;
        var newConversations = [];

        var stop = conversations.length;
        var index = 0

        //loop promise
        return promiseWhile(() => {
            // Condition for stopping
            return index < stop;
        }, () => {
            // Action to run, should return a promise
            return new Promise((resolve, reject) => {
                helperGetConvo(conversations[index].ConversID)
                    .then(function(convo) {
                            newConversations.push(createConversationObject({
                            messages: [],
                            name: convo.conversationName,
                            users: ['broulaye', 'doumbia'],
                            Id: convo.conversationID
                            }));
                            index++;
                            resolve();
                        })
                        .catch((error) => {
                           console.log('failure when finding conversation: ' + error);
                           index++;
                           resolve();
                        });
            })
        })
            //This will execute when loop ends
            .then(() => {
                return newConversations;
            });
    }

Hope this helps.

Upvotes: 1

Eric Guan
Eric Guan

Reputation: 15992

You need to use Promise.all

function getConversations(user){
    var conversations = user.Conversations
    var promises = conversations.map(c=>helperGetConvo(c.ConversID))

    return Promise.all(promises)
        .then(data=>{
            let newConversations = data.map(convo=>{
                return createConversationObject({messages:[], name:convo.conversationName, users:["broulaye", "doumbia"], Id:convo.conversationID})
            })
            return newConversations
        })
        .catch(reason=>{
            console.log("failure when finding conversation: " + reason)
        })
}

Use the function like so

getConversations(user).then(newConversations=>{
    //your code
})

Upvotes: 2

Jim B.
Jim B.

Reputation: 4694

One way is to collect the promises in an array using map instead of for-in. Then use Promise.all() to wait for all of them to resolve (or one to reject).

Something like:

return Promise.all(conversations.map(conversation => {
    return helperGetConvo(...).then().catch();
}

Remember all promises MUST either resolve or reject. If you don't follow this rule you get into trouble.

Upvotes: 1

Related Questions