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?
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?
Share Improve this question asked Aug 11, 2017 at 8:11 newer studentnewer student 951 gold badge3 silver badges10 bronze badges 6- Could you please reduce the problem to a specific question and provide a minimal reproducible example? – PeterMader Commented Aug 11, 2017 at 8:22
- you can answer only one question: does promise guarantee sync programing? – newer student Commented Aug 11, 2017 at 8:30
- No, of course not. Promises are a just a better way to deal with asynchrony. You shouldn't try to make asynchronous tasks synchronous. – PeterMader Commented Aug 11, 2017 at 8:32
-
avoid the Promise Constructor Anti-Pattern -
getPostAuthorNamereturns a Promise, no need to wrap the call in a promise ... and in turn,findOneandmongo.connectalso return Promises, so, no need to wrap those in a Promise constructor either – Jaromanda X Commented Aug 11, 2017 at 8:35 -
does promise guarantee sync programing- quite the opposite, promises guarantee asynchronous programming – Jaromanda X Commented Aug 11, 2017 at 8:37
3 Answers
Reset to default 3Don'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 plex 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;
});
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');
}
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
postsisn'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 =>