0

I just started using ElasticSearch in Node/Express, and not being an experienced JavaScript/ES6 developer I'm a bit confused by Promises.

The situation is as follows: I receive a request which includes one or more query parameters. These need to be translated to an ElasticSearch query. Some of the query parameters can readily be translated in a (part of the) ElasticSearch query, some others require longer running tasks such as querying another ElasticSearch index.

This is what I have in my service layer for now:

const find = function(q) {
    return generateElasticQuery(q).then(function(elasticQuery) {
        return model.find(elasticQuery)
    })
}

where q contains the query parameters from the GET request.

In generateElasticQuery I initiate a query object and then modify it using a a number of functions:

const generateElasticQuery = function(q) {
    let elasticQuery = { "term": {} }
    return Promise.all([
        processParameterA(q, elasticQuery),
        processParameterB(q, elasticQuery)
    ]).then(function() {
        return elasticQuery
    })
}

These functions look somewhat like this:

const processParameterA = function(q, elasticQuery) {
    return new Promise(function(resolve, reject) {
        if (q.parameterA) {
            someOtherAsyncTask(q.parameterA).then(function(res) {
                // modify elasticQuery
                resolve()
            })
        } else {
            resolve()
        }
    })
}

const processParameterB = function(q, elasticQuery) {
    return new Promise(function(resolve, reject) {
       // nothing to be done for now
       resolve()
    })
}

This all seems to work but it doesn't feel quite right. Is there a better approach for this situation?

Edit 1:

So after Bergi's comment I first refactored to this:

const processParameterA = function(q) {
    return new Promise(function(resolve, reject) {
        if (q.parameterA) {
            someOtherAsyncTask(q.parameterA).then(function(res) {
              resolve(res)
            })
        } else {
            resolve()
        }
    })
}

const generateElasticQuery = function(q) {
    return Promise.all([
        processParameterA(q),
        processParameterB(q)
    ]).then(function(results) {
        let parameterAResult = results[0]
        let parameterBResult = results[1]
        let elasticQuery = { "term": {} }
        // modify elasticQuery
        return elasticQuery
    })
}

And then to:

const generateElasticQuery = function(q) {

    let tasks = []
    if (q.parameterA) {
        tasks[0] = someOtherAsyncTask(q.parameterA)
    }

    return Promise.all(tasks).then(function(results) {
        let parameterAResult = results[0]
        let elasticQuery = { "term": {} }
        // modify elasticQuery
        return elasticQuery
    })
}
user3170702
  • 1,971
  • 8
  • 25
  • 33
  • Try not to modify `elasticQuery` in the `processParameter` functions, but resolve with the parts that you want to add and then construct the query from all the parts in the `then` callback after the `Promise.all`. – Bergi May 18 '17 at 14:18
  • Also, avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi May 18 '17 at 14:19
  • 1
    As Bergi mentioned you hardly ever need `new Promise` call. For example: `const processParameterA = q => q.parameterA ? someOtherAsyncTask(q.parameterA) : Promise.resolve()` BTW `Promise.all` works with non-promises `Promise.all([1, Promise.resolve(2)]).then(x => console.log(x))` – Yury Tarabanko May 18 '17 at 15:24

0 Answers0