How do I rewrite a series of conditional statement

2020-04-12 08:49发布

问题:

exports.create = function(req, res) {
  var company_id = req.company_id;
  var client = new Client(req.body);

  Company.findOne({_id: company_id}, function(err, company) {
    if(err) {
      response = {
        status: 'error',
        error: err
      }

      return res.json(response);
    } else if(!company) {
      response = {
        status: 'error',
        error: 'Invalid company_id'
      }

      return res.json(response);
    } else {
      client.save(function(err) {
        if(err) {
          response = {
            status: 'error',
            error: err
          }
        } else {
          response = {
            status: 'ok',
            client: client
          }
        }

        return res.json(response);
      });
    }
  });
}

That's my code (using Express, if it matters). I'm trying to learn more about promises, specifically with Q. I feel that this is the perfect kind of logic that can be implemented with promises to avoid this hairy conditional nest. But I'm not sure how to start?

回答1:

But I'm not sure how to start?

Start by removing the callbacks and using the Promise methods instead. Then put the error handling in a dedicated error callback, instead of using that condition. Also, you can put the code for building that response object in the very end (removing the duplication), and only pass/throw the err down to it.

exports.create = function(req, res) {
  var client = new Client(req.body);

  Q.ninvoke(Company, "findOne", {_id: req.company_id}).then(function(company) {
    if(!company)
      throw 'Invalid company_id';
    else
      return company;
  }).then(function(company) {
    return Q.ninvoke(client, "save");
  }).then(function(saveResult) {
    return {
      status: 'ok',
      client: client
    };
  }, function(err) {
    return {
      status: 'error',
      error: err
    };
  }).done(function(response) {
    res.json(response);
  });
};


回答2:

This is not really the use case for Promises. Promises are a method for eliminating deeply nested callbacks in node.js, not complex logic dealing with a result, like you have in your code. Think of the case where the outcome of your .findOne query would determine the next query you need to make - this quickly leads to a nested callback situation, which can be made less nested and more natural to read, evaluate, re-factor, and build upon if you use Promises.

You do have one nested callback for the outcome of your .save call. So, you might successfully re-factor this using Promises, like so

new Promise(function() {
    Company.findOne()
    ...
.then(function(company) {
    company.update()
    ...
}, function(err) {
    ...
})

The starting point is to wrap your first query so that it returns a promise instead of dealing with callbacks within callbacks.