Promises structure misunderstood

Question

I have a problem with understanding Promises syntax.

So, what I am trying to do:

getPosts() gets some data from a DB then, I want to get some metadata for each row with another promise call, addMetadata(). Then, once all the metadata is fetched, I want to console it out.

See my attempt below:

var getPosts = function(){

    return new Promise(function(resolve, reject){
        postModel.find()
        .exec(function(err, posts) {
            resolve(posts);
        );
    });
};

var addMetadata = function(posts){

    var options = {
        host: 'localhost',
        port: 3000,
        path: '',
        method: 'GET'
    };

    var postPromises = posts.map(function(post) {
        return new Promise(function(resolve) {
            options.path = '/api/user?id=' + post.profileID;
            var req = http.get(options, function(res) {
                var bodyChunks = [];
                res.on('data', function(chunk) {
                    bodyChunks.push(chunk);
                }).on('end', function() {
                    var body = Buffer.concat(bodyChunks);
                    var parsedBody = JSON.parse(body);
                    post.fullname = parsedBody.user.fullname;
                    post.profilePic = parsedBody.user.profilePic;

                    // resolve the promise with the updated post
                    resolve(post);
                });
            });
        });
});

// Is this the right place to put Promise.all???
Promise.all(postPromises)
.then(function(posts) {
    //What should I put here
});
};


getPosts()
.then(function(posts){
    return addMetadata(posts);
})
.then(function(posts){//I get a little lost here
    console.log();//posts is undefined
});

Of course, my understanding is wrong but I thought I was going the right way. Can someone please guide me to the right direction?

Thanks

Could you try to better format your code to make it easier to understand? — sp00m, Jul 12 '17 at 14:57
hard to tell because of the formatting as @sp00m pointed out, but you might be missing return statements in addMetadata — terpinmd, Jul 12 '17 at 14:59
https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call — Benjamin Gruenbaum, Jul 12 '17 at 14:59
@BenjaminGruenbaum - That is a generic Promise article. As you can see, I am getting the hang of it but I can't find where I have gone wrong or where Promise.all goes as I am using a combination of .then and Promise.all — Sean, Jul 12 '17 at 15:03
@ShayanKhan I don't think you understand how control flows in a JavaScript program, try `Promise.all(postPromises).then(addMetadata).then(console.log)` — Benjamin Gruenbaum, Jul 12 '17 at 15:05
@BenjaminGruenbaum - can you elaborate please? Where is this Promise.all supposed to go? If it is in the main .then (after getPosts), then postPromises will be inaccessible. If this is within addMetadata, then how can I call .then(addMetadata)? — Sean, Jul 12 '17 at 15:10

score 3 · Accepted Answer · answered Jul 12 '17 at 15:17

3

Change

// Is this the right place to put Promise.all???
Promise.all(postPromises)
    .then(function (posts) {
        //What should I put here
    });

into

// Is this the right place to put Promise.all???
return Promise.all(postPromises);

This way your addMetadata function will return Promise that resolve when all promises from postPromises resolves or reject if any of postPromises rejects.

answered Jul 12 '17 at 15:17

ponury-kostek

7,824
4
23
31

That did it mate! Thanks a lot – Sean Jul 12 '17 at 15:20

Custodio · Answer 2 · 2017-07-12T15:16:24.083

2

The key point to understand the async concept of it and what time the content is available.

Reading this will help to put you in the right direction.

For instance:

var promise = new Promise(function(resolve, reject) {
    resolve(1);
});

promise
.then(function(val) {
    console.log(val); // 1
    return val + 2;
})
.then(function(val) {
    console.log(val); // 3
})

After as per your scenario, in order to have all the metadata Promise.all is the way to go.

Promise.all(arrayOfPromises).then(function(arrayOfResults) {
   // One result per each promise of AddMetadata
})

edited Jul 12 '17 at 15:16

answered Jul 12 '17 at 15:13

Custodio

8,594
15
80
115

I actually understand this part but thanks for refreshing my memory. The problem is with the looped Promises I have and where I put Promise.all – Sean Jul 12 '17 at 15:15
Cool, then yeah, you got to use `Promise.all`. Try to use a `map` in your list to create a list of promises and then get all the results. – Custodio Jul 12 '17 at 15:17
Thanks for the comments mate – Sean Jul 12 '17 at 15:21

score 0 · Answer 3 · answered Jul 12 '17 at 15:11

0

What you wanna do here, if I am correct, is called streams, as you wanna call multiple paralel promises as your concept of looping through list of posts using map is not going to work this way Take a look at this short video introducing streams Streams - FunFunFunction, he is using library for workin with streams called Baconjs

Here is a short example on streams

const stupidNumberStream = {
    each: (callback) => {
    setTimeout( () => callback(1), 3000 )
    setTimeout( () => callback(2), 2000 )
    setTimeout( () => callback(3), 1000 )
  }
}
stupidNumberStream.each(console.log)

answered Jul 12 '17 at 15:11

Pavel Hasala

946
10
14

Actually, this is not a must for me to go the route I have gone. I just need a suggestion on how do I structure my promises (I get the concept). As you can see, I am almost there but I am messing up somewhere. Being fairly new to JS doesn't help either. – Sean Jul 12 '17 at 15:14
you made a lot of changes there while i was writing it :-) – Pavel Hasala Jul 12 '17 at 15:26
See the first comment, they pointed out how badly I've posted this question lol. I was losing my mind. And thanks for suggesting Streams. I'll have a look at that too. You never know when these things come in handy – Sean Jul 12 '17 at 15:30
yeah, that mislead me :) – Pavel Hasala Jul 12 '17 at 15:32
My bad, sorry mate – Sean Jul 12 '17 at 15:38

score 0 · Answer 4 · answered Jul 12 '17 at 15:17

Your getPosts function is good in the sense that its only job is to promisfy the database find. (Though, I think if it's mongo, the exec produces a promise for you).

Your addMetadataToAPost is less good, because it mixes up processing an array of posts and "promisifying" the http.get. Use the same pattern you applied correctly in the first function and return a promise to do a single get and add metadata. (It would be even better to just wrap the get, which you can reuse, and build a simple add-metadata function that returns - rather than creates - a promise)

// renamed pedantically
var addMetadataToAPost = function(post) {
    return new Promise(function(resolve) {
        options.path = '/api/user?id=' + post.profileID;
        var req = http.get(options, function(res) {
            var bodyChunks = [];
            res.on('data', function(chunk) {
                bodyChunks.push(chunk);
            }).on('end', function() {
                var body = Buffer.concat(bodyChunks);
                var parsedBody = JSON.parse(body);
                post.fullname = parsedBody.user.fullname;
                post.profilePic = parsedBody.user.profilePic;

                // resolve the promise with the updated post
                resolve(post);
            });
        });
    });
}

Now your batching function is simple:

// also renamed pedantically
var addMetadataToAllPosts = function(posts){
    var postPromises = posts.map(function(post) {
        return addMetadataToAPost(post)
    })
    return Promise.all(postPromises)
};

Your original code should work...

getPosts().then(function(posts){
    return addMetadataToAllPosts(posts);
})
.then(function(posts){
    console.log(posts);//posts should be an array of posts with metadata added
});

Promises structure misunderstood

4 Answers4