Reputation: 95
hi i'm trying to synchronize my functions with convert callback to promise.
i want to add to all posts, post.authorName field via forEach loop and query to user collection.
first i tried with callbacks but this is async and i need to a sync tool.
so i use promise but still my result is like callback.
this is my code:
var mongo = require('mongodb').MongoClient();
var url = "mongodb://localhost:27017/blog";
var ObjectId = require('mongodb').ObjectID;
var listPosts = function(req, res) {
find('post', {}, 10, {author: 1})
.then(function(posts) {
var myPosts = posts;
const promises = [];
myPosts.forEach(function(post) {
console.log("hi i'm forEach" + '\n');
console.log(post);
console.log('\n');
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
})
resolve();
});
console.log("i'm end of forEach and this is result:");
console.log(post);
console.log('\n');
promises.push(promise);
});
Promise.all(promises).then(() => {
console.log('i should print at end' + '\n');
});
});
}
var getPostAuthorName = function(authorID) {
return new Promise(function(resolve, reject){
findOne('user', {_id: new ObjectId(authorID)})
.then(function(result){
console.log("i'm getPostAuthorName" + '\n');
resolve(result.name);
})
})
}
var find = function(collection, cond = {}, limit = 0, sort = {}) {
return new Promise(function(resolve, reject){
mongo.connect(url)
.then(function(db){
db.collection(collection)
.find(cond).limit(limit).sort(sort).toArray()
.then(function(result){
resolve(result);
})
})
});
}
var findOne = function(collection, cond = {}){
return new Promise(function(resolve, reject){
mongo.connect(url)
.then(function(db){
db.collection(collection).findOne(cond)
.then(function(result){
console.log("i'm findOne" + '\n');
resolve(result);
})
})
})
}
listPosts();
and at the end i recieve this result:
hi i'm forEach
{ _id: 59888f418c107711043dfcd6,
title: 'FIRST',
content: 'this is my FIRST post',
timeCreated: 2017-08-07T16:03:13.552Z,
authorID: '5987365e6d1ecc1cd8744ad4' }
i'm end of forEach and this is result:
{ _id: 59888f418c107711043dfcd6,
title: 'FIRST',
content: 'this is my FIRST post',
timeCreated: 2017-08-07T16:03:13.552Z,
authorID: '5987365e6d1ecc1cd8744ad4' }
hi i'm forEach
{ _id: 598d60d7e2014a5c9830e353,
title: 'SECOND',
content: 'this is my SECOND post',
timeCreated: 2017-08-07T16:03:13.552Z,
authorID: '5987365e6d1ecc1cd8744ad4' }
i'm end of forEach and this is result:
{ _id: 598d60d7e2014a5c9830e353,
title: 'SECOND',
content: 'this is my SECOND post',
timeCreated: 2017-08-07T16:03:13.552Z,
authorID: '5987365e6d1ecc1cd8744ad4' }
i should print at end
i'm findOne
i'm getPostAuthorName
i'm findOne
i'm getPostAuthorName
why functions don't run synchronously. what's the solution?
Upvotes: 3
Views: 17040
Reputation: 7285
Don't create promises if you don't need them! Instead, make use of the ability to chain promises:
var mongo = require('mongodb').MongoClient();
var url = "mongodb://localhost:27017/blog";
var ObjectId = require('mongodb').ObjectID;
var listPosts = function () {
return find('post', {}, 10, {author: 1})
.then(function (posts) {
var promises = posts.map(post => getPostAuthorName(post.authorID));
return Promise.all(promises).then(names => names.map((name, index) => {
var post = posts[index];
post.authorName = name;
return post;
});
});
};
var getPostAuthorName = function(authorID) {
return findOne('user', {_id: new ObjectId(authorID)}).then(author => author.name);
}
var find = function(collection, cond = {}, limit = 0, sort = {}) {
return mongo.connect(url)
.then(db => db.collection(db)
.find(cond)
.limit(limit)
.sort(sort)
.toArray()
);
};
var findOne = function(collection, cond = {}) {
return mongo.connect(url).then(db => db.collection(db).findOne(cond));
};
listPosts().then(posts => console.log('Post:', post, ', author: ', post.authorName));
Creating unnecessary promises using the new Promise
constructor is called the explicit-construction anti-pattern.
But that wasn't the only issue in your code: in unnecessary promise in the following snippet made the code so complex that you didn't realize that you resolved the promise before the author's name was found:
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
})
resolve(); // why resolve immediately?
});
Instead, it should have been like this:
const promise = getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
});
Upvotes: 3
Reputation: 1
your issue is with this (badly indented) code
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
})
resolve();
});
Properly indented it looks like
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
})
resolve();
});
So it's clear that resolve
is called "synchronously" with respect to getPostAuthorName
- but before the .then
of getPostAuthorName
(which is asynchronously called) could be possibly called - hence why your promises
array is all resolved too early
so, if you move it
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
resolve();
})
});
Now, your code should behave as you expect
Addressing the "promise constructor anti-patterns" in your code - of which the above is an example
Since getPostAuthorName
returns a Promise, there's no need to do
const promise = new Promise(function(resolve, reject){
getPostAuthorName(post.authorID)
.then(function(postAuthor){
post.authorName = postAuthor;
resolve(); // resolves to "undefined"
})
});
This is equivalent to
const promise = getPostAuthorName(post.authorID).then(function(postAuthor){
post.authorName = postAuthor;
return; // returns "undefined", just like your resolve() results in
});
So, removing all those anti-patterns, and using
Promise.all(posts.map(
instead of building an array with push
would result in code like
const mongo = require('mongodb').MongoClient();
const url = "mongodb://localhost:27017/blog";
const ObjectId = require('mongodb').ObjectID;
const listPosts = function(req, res) {
find('post', {}, 10, {author: 1})
.then(posts =>
Promise.all(posts.map(post =>
getPostAuthorName(post.authorID)
.then(postAuthor => post.authorName = postAuthor)
))
)
.then(() => console.log('i should print at end' + '\n'));
}
const getPostAuthorName = authorID =>
findOne('user', {_id: new ObjectId(authorID)})
.then(result => result.name);
const find = (collection, cond = {}, limit = 0, sort = {}) =>
mongo.connect(url)
.then(db =>
db.collection(collection)
.find(cond)
.limit(limit)
.sort(sort)
.toArray()
);
const findOne = (collection, cond = {}) =>
mongo.connect(url)
.then(db =>
db.collection(collection)
.findOne(cond)
);
I think I fell into the trap again .. I bet
posts
isn't a javacript Array - in that case I would make a function like
const makeArray = collection => {
const ret = [];
collection.forEach(item => ret.push(item));
return ret;
};
and change
Promise.all(posts.map(post =>
to
Promise.all(makeArray(posts).map(post =>
Upvotes: 0
Reputation: 379
If you want to convert a callback to a promise, you can simply make something like that :
function functionWithCallback(params, callback)
{
[...]
callback(true);
}
function functionWithPromise(params)
{
return new Promise((resolve, reject) => {
functionWithCallback(params, (done) => {
if (done)
return resolve(true);
reject(false);
});
});
}
Now, you can synchronize promises with the await keyword (don't forget to put your function async). Example :
async function main()
{
const p1 = functionWithPromise('1');
const p2 = functionWithPromise('2');
await p1;
await p2;
console.log('End');
}
Upvotes: 2