Блог

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

 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));
});
  

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

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

Чтобы передать несколько переменных / Promises следующему .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)`
  

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

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

 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! 
});
  

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

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

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

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

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

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

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

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

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

Я вижу 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. Во время цепочки обработчик получает некоторую информацию, которую вы не можете легко распространить по цепочке. Например, когда вам нужны две разные части информации, которые поступают из базы данных, и вам нужны обе в конце для выполнения работы.

 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
})