Является ли использование обещания внутри другого обещания антипаттерном?

Question

У меня есть код, который выглядит следующим образом:

app.post("/api/exercise/add", function(req, res, next) {
User
 .findById(req.body.userId)
 .exec()
 .then(user => user)
 .then(function(user) {
   let exercise = new Exercise({
     description: req.body.description,
     duration: req.body.duration,
     date: req.body.date, //BUG: must add validations, date accepts 19984-01-01
     user: user
   })
   .save()
   .then(function(exercise) {
     user.exercises.push(exercise)
     user.
      save().
      then(user => res.json({ status: 201, exercises: user.exercises }))
  })
  .catch(err => next(err))
})
.catch(err => next(err));
});

Является ли тот факт, что я использую обещание внутри другого обещания, в данном случае, считается анти-паттерном?

Answer 1

В некотором смысле это не элегантно - проблема в том, что оно создает ненужное .then вложение. Если обработчики .then и .catch, которые следуют за обоими Обещаниями, одинаковы, вы можете просто return новое Обещание внутри .then, чтобы передать его на следующие .then или .catch, как в коде ниже.

Чтобы передать несколько переменных / обещаний следующей .then без переназначения внешней переменной, используйте Promise.all:

app.post("/api/exercise/add", function(req, res, next) {
User
 .findById(req.body.userId)
 .exec()
 .then(function(user) {
   // return the Promise so it can be used by the next then, without nesting
   // because you also need access to `user` in the next then, use Promise.all
   return Promise.all([user, new Exercise({
     description: req.body.description,
     duration: req.body.duration,
     date: req.body.date, //BUG: must add validations, date accepts 19984-01-01
     user: user
   })
   .save()]);
  })
  .then(function([user, exercise]) {
     user.exercises.push(exercise);
     // return the Promise so it can be used by the next then, without nesting:
     return user.save();
   })
   .then(user => res.json({ status: 201, exercises: user.exercises }))
   .catch(err => next(err));
});

Обратите внимание, что

.then(user => user)`

совершенно лишний - он ничего не делает, у вас уже есть Обещание, которое разрешается до user, которое вы хотите в следующем .then.

Answer 2

Это не обязательно антипаттерн, но многое зависит от того, зачем вы это делаете.

Может быть веская причина для разрыва цепи и запуска новой, но если вы обнаружите, что делаете этослишком часто, тогда что-то не так, и, вероятно, вам следует переосмыслить свой поток.

Я вижу две распространенные причины, по которым люди склонны начинать новую цепочку

1. Обработчик в какой-то момент во время создания цепочкирешение, основанное на условии, и у каждой ветви есть свой способ работы.На данный момент совершенно правильно начать новую цепочку, но я бы создал новый метод, который возвращает обещание.Следующий обработчик в цепочке должен знать о том, что он может получать разнородные данные

NewPromise()
.then( res => {
  if (someCond) {
    return OtherPromise(args)
  }
  ....
  return obj
})
.then( res => {
  //this promise must be aware that res may be heterogeneous
})

2. Во время цепочки обработчик получает некоторую информацию, которую вы не можете легко распространить по цепочке.Например, когда вам нужны 2 разные части информации, поступающей из БД, и вам нужно оба в конце, чтобы выполнить работу.

User.findById(uid1)
.then(user1 => {
  return User.finById(uid2)
})
.then(user2 => {
 // at this point user1 is not available any more
})

Решением для этого является наличие переменной вне цепочки, а неначать новую цепочку

var user1

User.findById(uid1)
.then(user => {
  user1 = user
  return User.finById(uid2)
})
.then(user2 => {
 // at this point user is available and has the value of user1
})

Answer 3

Ваш поток выполнения теперь разделен на несколько ветвей, что может быть желательным поведением.

При написании кода вы всегда должны думать о возможности повторного использования и удобочитаемости.

Как другой программист мог бы читать и понимать мой код легко и без головной боли?

Трудно понять, как вы его составляете.Вы должны поместить асинхронное действие, которое вы хотите выполнить, в отдельную функцию.

Разделение сложных элементов на функции - это хорошая практика, которую следует использовать в целом, а не только в этом конкретном случае.Попробуйте использовать одну функцию для выполнения одной задачи и один поток выполнения.

User
 .findById(req.body.userId)
 .exec()
 .then(user => user)
 .then(user => asynchronousAddUser(user))
 .catch(err => next(err));

Answer 4

У нас может быть что-то вроде этого:

new Promise((resolve, reject) => { 
  let x = 25; 
  if (x%2 === 0) {
    return Promise.resolve('even');
  } else {
    return Promise.resolve('odd');
  }
})
.then(result => {
  console.log('the number is '+result);
});

В этом случае обе ветви условия являются однородными, они оба возвращают строку, а результат обрабатывается одинаково.

Но это не всегда происходит, например:

new Promise((resolve, reject) => {
  if (user.type === 'admin') {
    return this.userService.getAdminTools();
  } else {
    return this.userService.getUserTools();
  }
})
.then(result => {
  // What type is the result? Maybe in this case, chaining is not the best solution! 
});

Если у вас больше ветвей и результат не однородный, возможно, цепочка не лучший выбор.Вы можете прослушать Обещание внутри другого Обещания или просто вызвать другой метод, содержащий асинхронный код