Есть несколько вещей, которые вы можете сделать с помощью вложенных боковых пирамид. Один из довольно универсальных методов - сначала обработать ошибки , а затем выполнить обычный код. Итак, допустим, у вас есть:
if (everythingFine) {
//do normal operation
} else {
//handle error
}
Это OK , но вы действительно столкнетесь с проблемами, как только получите
if (everythingFine) {
//do operation
if (operationSuccessful) {
//do next step
} else {
//handle error with operation
}
} else {
//handle error
}
Вы сразу начнете складывать больше и больше блоков. Более того, логика c для успеха, как правило, очень большая, так как вам нужно будет сделать больше вещей позже. И logi c обработки ошибок может быть одной строкой. Так что, если вы хотите узнать, что происходит, при ошибке вам нужно много прокрутить.
Чтобы избежать этого, вы можете выйти рано. Шаги для преобразования вашего кода одинаковы:
- Переверните условие
if
, которое проверяет на успешность, вместо этого проверьте на ошибки - Поменяйте местами блок
if
и else
block. - Добавьте
return
к if
. - Снимите блок
else
и сгладьте его.
И вы получите следующее
if (!everythingFine) {
//handle error
return;
}
//do operation
if (!operationSuccessful) {
//handle error with operation
return;
}
//do next step
Теперь вы удаляете вложенность и отбрасываете блоки else, поскольку вы просто набираете return
и выходите из функции. Если ваша обработка ошибок - это оператор throw
, который уже существует, значит, вам не нужно return
.
Следующее, что у вас есть несколько дублирующих логи c здесь:
if (destroyerUser.role == "Admin") {
Cyclic.deleteOneById({ _id: user._id }, (err) => {
if (err) {
res.json({ success: false, msg: "Error deleting user" });
} else {
res.json({ success: true, msg: "Successfully deleted user" });
}
})
} else if (destroyerUser.role == "Moderator") {
if (user.role == "Moderator" || user.role == "Admin") {
res.json({ success: false, msg: "You don't have sufficient permissions" })
} else {
Cyclic.deleteOneById({ _id: user._id }, (err) => {
if (err) {
res.json({ success: false, msg: "Error deleting user" });
} else {
res.json({ success: true, msg: "Successfully deleted user" });
}
})
}
}
Независимо от того, является ли destroyerUser
администратором или модератором, вы в любом случае называете одинаковым deleteByOne
. И вы также возвращаете одно и то же сообщение об успехе в обоих случаях. Итак, у вас есть шесть различных веток в коде, тогда как у вас есть только три результата:
+----------------+-----------+-----------------+-----------------------------------------+
| destroyer role | user role | deletion result | operation result |
+----------------+-----------+-----------------+-----------------------------------------+
| Moderator | Moderator | N/A | "You don't have sufficient permissions" |
| Moderator | Admin | N/A | "You don't have sufficient permissions" |
| Moderator | <other> | success | "Successfully deleted user" |
| Moderator | <other> | error | "Error deleting user" |
| Admin | <any> | success | "Successfully deleted user" |
| Admin | <any> | error | "Error deleting user" |
+----------------+-----------+-----------------+-----------------------------------------+
Вместо этого вы можете просто проверить, выполняет ли модератор удаление сначала, и если они делают имеют достаточные права, они администратор, затем просто выполните удаление один раз:
if (destroyerUser.role == "Moderator" && (user.role == "Moderator" || user.role == "Admin")) {
res.json({ success: false, msg: "You don't have sufficient permissions" })
return;
}
Cyclic.deleteOneById({ _id: user._id }, (err) => {
if (err) {
res.json({ success: false, msg: "Error deleting user" });
return;
}
res.json({ success: true, msg: "Successfully deleted user" });
})
Итак, если мы применим эти две вещи:
- инвертируем логи c и досрочно выйдите
- удалите дублирующий код
Тогда ваш код будет выглядеть так:
router.delete("/user/:username", passport.authenticate("jwt", { session: false }), (req, res) => {
var { username = null } = req.params;
if (username == null) {
res.json({ success: false, msg: "Username is not valid" });
return;
}
User.getUserById(req.user.id, (err, destroyerUser) => {
if (err) {
res.json({ success: false, msg: "User not found" });
return;
}
if (!destroyerUser) {
res.json({ success: false, msg: "Destroyer user not found" });
return;
}
if (destroyerUser.role !== "Admin" && destroyerUser.role !== "Moderator") {
res.json({ success: false, msg: "You don't have sufficient permissions" })
return;
}
User.getUserByUsername(username, (err, user) => {
if (err) {
res.json({ success: false, msg: "Error finding user" });
return;
}
if (!user) {
res.json({ success: false, msg: "User not found" });
return;
}
if (user._id.toString() === destroyerUser._id.toString()) {
res.json({ success: false, msg: "You can't delete yourself" });
return;
}
if (destroyerUser.role == "Moderator" && (user.role == "Moderator" || user.role == "Admin")) {
res.json({ success: false, msg: "You don't have sufficient permissions" })
return;
}
Cyclic.deleteOneById({ _id: user._id }, (err) => {
if (err) {
res.json({ success: false, msg: "Error deleting user" });
return;
}
res.json({ success: true, msg: "Successfully deleted user" });
})
})
});
})
Одна заметка - я преобразовал условие destroyerUser.role == "Admin" || destroyerUser.role == "Moderator"
to destroyerUser.role !== "Admin" && destroyerUser.role !== "Moderator"
это потому, что мне не нравится логическое НЕ перед длинными выражениями:
if (!(destroyerUser.role == "Admin" || destroyerUser.role == "Moderator"))
Легко пропустить, потому что сначала вы видите OR и два выражения. К счастью, это очень легко изменить, используя закон Де Моргана для логических выражений not (A or B) = not A and not B
. Следовательно, как я изменил это.
Вы можете еще больше уменьшить дублирование кода, добавив новую функцию для обработки всех ошибок. Во всех случаях вы делаете то же самое, кроме передачи другого сообщения, поэтому вы можете просто получить:
const error = msg => res.json({ success: false, msg });
Итак, вы можете позвонить, например, error("User not found")
. Это на самом деле не уменьшает объем кода, который у вас есть, но он более последовательный. Кроме того, если вы решите добавить дополнительные сведения в ответ об ошибке (например, отправить дополнительные заголовки или попытаетесь перевести сообщение об ошибке), у вас есть центральное расположение для него.