javascript - Mongoose query promise in function make code became ugly -


i creating users api, want check if username had been used. wrote static function

 static findbyname(name) {    const query = user.where({      username: name,    });    query.findone((queryerr, user) => {      if (queryerr) {        console.log(queryerr);        return false;     }     return user;    });  } 

when called in signup

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

findbyname return undefined. use promise.

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

that horrible code. there better way clean code?

is there better way clean code

yes. going assume /signup defined post route on usual express app instance

with said you, since using promises, can go step further , use async/await enabled default in node.js v7.6+.

this make code read more synchronously:

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

you may have noticed use of try/catch. because since not using .catch() , still have handle error occurs. further clean code, can write error handler middleware take care of errors us:

src/middleware/error-handlers.js

// wraps router handler, catches errors, , forwards next middleware handles errors exports.catcherrors = action => (req, res, next) => action(req, res).catch(next);  // notice first parameter `error`, means 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,     },   }); }; 

now need use our error handlers:

app.js

const { catcherrors, displayerrors } = require('./middleware/error-handlers')  // whenever defined function, needs have `async` keyword async function signup(req, res) { ... }  // wrap function call app.post('/signup', catcherrors(signup))  // handle errors app.use(displayerrors) 

using above middleware transforms our code to:

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

you can see how code easier read without noise.

be sure read on:


Comments