函数中的猫鼬查询承诺使代码变得丑陋

Question

我正在创建我的用户API，我想检查用户名是否被使用过。 所以我写了一个静态函数

static findByName(name) {
const query = User.where({
username: name,
});
query.findOne((queryErr, user) => {
if (queryErr) {
console.log(queryErr);
return false;
}
return user;
});
}

当我在注册中调用它时

signup(req, res) {
if (!req.body.username || !req.body.password || !req.body.email) {
return res.status(400).json({ success: false, message: 'Bad Request' });
}
if (!Users.findByName(req.body.username)) {
return res.status(409).json({ success: false, message: 'Username has been used' });
}
const hashedPassword = this.genHash(req.body.password);
const newUser = User({
username: req.body.username,
});
}

查找按名称返回未定义。 最后我用承诺。

signup(req, res) {
if (!req.body.username || !req.body.password || !req.body.email) {
return res.status(400).json({ success: false, message: 'Bad Request' });
}
return Users.findByName(req.body.username).then((existingUser) => {
if (existingUser) {
return res.status(409).json({ success: false, message: 'Username has been used' });
}
const hashedPassword = this.genHash(req.body.password);
const newUser = User({
username: req.body.username,
password: hashedPassword,
email: req.body.email,
});
return newUser.save().then((user) => {
res.json({ success: true, user });
}).catch((err) => {
res.status(500).json({ success: false, message: 'Internal Server Error' });
});
}).catch((err) => {
res.status(500).json({ success: false, message: 'Internal Server Error' });
});
}

这真是可怕的代码。 有没有更好的方法来清理代码？

Answer 1

有没有更好的方法来清理代码

是的。我将假设/signup被定义为通常的 Expressapp实例上的POST路由

话虽如此，由于您已经在使用承诺，您可以更进一步，使用 Node.js v7.6+ 中默认启用的async/await。

这将使您的代码更同步地读取：

async signup(req, res) {
if (!req.body.username || !req.body.password || !req.body.email) {
return res.status(400).json({ success: false, message: 'Bad Request' });
}
try {
const existingUser = await Users.findByName(req.body.username)
if (existingUser) {
return res.status(409).json({ success: false, message: 'Username has been used' })
}
const hashedPassword = this.genHash(req.body.password);
const newUser = await User({
username: req.body.username,
password: hashedPassword,
email: req.body.email,
}).save()
res.json({ success: true, newUser });
} catch (error) {
res.status(500).json({ success: false, message: 'Internal Server Error' });
}
}

您可能已经注意到try/catch的使用。这是因为由于我们不使用.catch()并且我们仍然必须处理发生的任何错误。为了进一步清理代码，我们可以编写一个错误处理程序中间件来为我们处理错误：

src/middleware/error-handlers.js

// Wraps the router handler, catches any errors, and forwards to the next middleware that handles errors
exports.catchErrors = action => (req, res, next) => action(req, res).catch(next);
// Notice the first parameter is `error`, which means it handles errors.
exports.displayErrors = (error, req, res, next) => {
const err = error;
const status = err.status || 500;
delete err.status;
err.message = err.message || 'Something went wrong.';
if (process.env.NODE_ENV === 'production') {
delete err.stack;
} else {
err.stack = err.stack || '';
}
res.status(status).json({
status,
error: {
message: err.message,
},
});
};

现在我们只需要use错误处理程序：

app.js

const { catchErrors, displayErrors } = require('./middleware/error-handlers')
// Whenever you defined the function, needs to have the `async` keyword
async function signup(req, res) { ... }
// Wrap the function call
app.post('/signup', catchErrors(signUp))
// Handle any errors
app.use(displayErrors)

使用上述中间件将我们的代码转换为：

async signup(req, res) {
const error = new Error()
if (!req.body.username || !req.body.password || !req.body.email) {
error.status = 400
error.message = 'Bad Request'
throw error
}
const existingUser = await Users.findByName(req.body.username)
if (existingUser) {
error.status = 409
error.message = 'Username has been used'
throw error
}
const hashedPassword = this.genHash(req.body.password);
const newUser = await User({
username: req.body.username,
password: hashedPassword,
email: req.body.email,
}).save()
res.json({ success: true, newUser });
}

您可以看到代码如何更容易阅读，而没有所有的噪音。

请务必阅读：

• 编写用于快速应用的中间件
• 快速错误处理

Answer 2

我无法测试我的解决方案是否实际有效(可能不起作用(或我是否涵盖了原始代码的所有功能。我试图做的是制作处理特定事物的函数，然后将它们链接在一起，以便您可以看到更清晰的程序流程。

signup(req, res) {
if (!req.body.username || !req.body.password || !req.body.email) {
return res.status(400).json({ success: false, message: 'Bad Request' });
}
const handleError = error => res.status(500).json({ success: false, message: 'Internal Server Error' });
const handleExistingUser = existingUser => {
if (existingUser) {
return res.status(409).json({ success: false, message: 'Username has been used' });
} else {
return Promise.resolve();
}
}
const handleCreateUser = () => {
const hashedPassword = this.genHash(req.body.password);
const newUser = User({
username: req.body.username,
password: hashedPassword,
email: req.body.email,
});
return newUser.save().then((user) => {
res.json({ success: true, user });
});
};
// the flow of the program is hopefully more clear here
return Users.findByName(req.body.username)
.then(handleExistingUser)
.then(handleCreateUser)
.catch(handleError);
}

如果你处理内外承诺的错误是一样的，那么我认为在外层处理错误就足够了。(在上一个示例中。但我不是100%确定。

Answer 3

你的函数返回undefined，因为它没有return语句。return user语句是findOne回调函数的(无用(返回值，而不是findByName的返回值。

如果你选择承诺，那么定义函数如下：

static findByName(name) {
return User.where({ username: name }).findOne().exec();
}

你的承诺链可以简化一点，像这样：

signup(req, res) {
function report(message, status = 200) {
res.status(status).json({ success: status === 200, message });
}
if (!req.body.username || !req.body.password || !req.body.email) {
return report('Bad Request', 400);
}
Users.findByName(req.body.username).then((existingUser) => {
return existingUser ? null // treat this condition in the next `then`
: User({
username: req.body.username,
password: this.genHash(req.body.password),
email: req.body.email,
}).save().exec();
}).then((user) => {
return existingUser ? report(user) 
: report('Username has been used', 409);
}).catch((err) => {
report('Internal Server Error', 500);
});
}