0

This function lists all collections in a MongoDB database, with the number of documents in each collection (bluebird promises).

function listMongoCollections(db) {
    var promises = []
    db.listCollections().toArray().then((docs) => {

        docs.forEach((doc) => {

            promises.push(
                new Promise((resolve) => {
                    db.collection(doc.name).count().then((count) => {
                        doc.count = count
                        resolve()
                    })
                })
            )
        })

        return Promise.all(promises)
    })
}

Is there a simpler way to do this? Using this method will flood an app with code, and I haven't even included error handling.

Pål Thingbø
  • 1,211
  • 1
  • 17
  • 17

1 Answers1

3

You can do a few things:

  • use .map to transform an array of collections into an array of promises, instead of .pushing into an array manually.
  • avoid wrapping promises in new Promises.
  • nest .then as little as possible. In the following example, notice how the first part just returns an array of promises. Only before returning the main function we wrap it in an .all promise.
function listMongoCollections(db) {
    const docs = db.listCollections().toArray().then(docs => {
        return docs.map(doc => {
            return db.collection(doc.name).count().then(count => {
                doc.count = count
                return doc;
            });
        });
    });
    return Promise.all(docs);
}

You can "simplify" it further by removing the returns and the intermediate docs constant.

function listMongoCollections(db) {
    return Promise.all(
        db.listCollections().toArray().then(docs => 
            docs.map(doc => 
                db.collection(doc.name).count().then(count => {
                    doc.count = count
                    return doc;
                })
            )
        )
    );
}

And perhaps with async/await we can make it even easier to read (although I'm not too familiar with it):

async function listMongoCollections(db) {
    let docs = await db.listCollections().toArray();
    docs = docs.map(async doc => {
        doc.count = await db.collection(doc.name).count();
        return doc;
    });
    return Promise.all(docs);
}

These are just examples, Mongo might offer even better solutions.

fregante
  • 29,050
  • 14
  • 119
  • 159
  • 3
    Since these are Bluebird promises, you can use [`.map()`](http://bluebirdjs.com/docs/api/map.html) and [`.all()`](http://bluebirdjs.com/docs/api/all.html) (see [this gist](https://gist.github.com/robertklep/f8f558b0125c950722629d0b76e4dd49)). – robertklep Jun 06 '16 at 12:27
  • 1
    Just a small critique, which is HIGHLY debatable, but IMO leaving out the `return`s definitely does not improve readability. At a quick glance it's as if the functions return nothing, until you realise there are no curly brackets. Anyway, this is probably more of a critique of the fat arrow syntax than your solution, but maybe you should add a note that the functions definitely DO have to return the promises. – Creynders Jun 06 '16 at 13:52
  • Totally, that's why I said and double-quoted _"simplify"_, not "improve readability". Robert's gist in the comment is actually a better solution – fregante Jun 06 '16 at 14:43
  • This is by far the best answer I've seen for promise antipatterns, anywhere. Thank you. – Pål Thingbø Jun 06 '16 at 19:33