Simple registration with NodeJS and Mongoose

Question

I'm new to Node.js (and also backend) for only a week, and now I'm trying to do a simple registration. I found that it's really hard to reduce complexity because pretty much every callback is error-first. Is there any way to reduce this? I found it seems impossible to avoid all of those if-elses.

const User = require('../../models/user')
const ERROR_MESSAGES = require('../../models/constant').ERROR_MESSAGES
const RESPONSE_STATUS = require('../../models/constant').RESPONSE_STATUS
const helper = require('../../helpers/helper')
const validator = require('validator')
module.exports.register = function(req, res) {
 const email = req.body.email
 const password = req.body.password
 const errors = []
 // input validation
 if (!email || email.trim().length == 0) {
 errors.push(ERROR_MESSAGES.EMPTY.EMAIL)
 } else if (!validator.isEmail(email)) {
 errors.push(ERROR_MESSAGES.INVALID.EMAIL)
 }
 if (!password) {
 errors.push(ERROR_MESSAGES.EMPTY.PASSWORD)
 }
 if (errors.length > 0) {
 return res.json(helper.generateResponse(errors, null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 User.findOne({email: email}, function(err, user){
 if (err) { 
 res.json(helper.generateResponse([ERROR_MESSAGES.UNKNOW], null, RESPONSE_STATUS.REQUEST_DENIED)) 
 } else if (user) {
 res.json(helper.generateResponse([ERROR_MESSAGES.DUPLICATE.EMAIL], null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 var user = new User()
 user.email = email
 user.password = user.generateHash(password)
 var errorValidation = user.validateSync();
 if (errorValidation) {
 // input validation
 errors = helper.getMongooseValidationErrors(errorValidation.errors)
 res.json(helper.generateResponse(errors, null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 user.save(function(error, newUser) {
 if (error) { 
 res.json(helper.generateResponse([error], null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 res.json(helper.generateResponse(errors, newUser, RESPONSE_STATUS.SUCCESS))
 }
 })
 }
 }
 })
 }
}

• James: Your code could certainly be tidied up, by how much depends on what version of Node your using?
• Amin Jafari: your code is not complex in my opinion, it is very clear what you're trying to do. However one method you can use to make it more clear is to divide your code piece by piece into methods or functions. It also helps avoid duplicate code.
• Rookie: @James I'm using v6.11.0.
• Rookie: @AminJafari I tried to do so, but it turns out that I need to write heaps of callback functions to achieve what I wanted to do.
• James: @AminJafari I disagree, this is classic callback hell

Answer 1

Well, the code is fairly simple, but you've made it complex due to a bad design. I would encourage the following:

• Use Promises. Mongoose implements promises and so does express, so most of the times you can avoid using callbacks. Also implement your own helpers with promises. Further more you can read up on Async/Await too.

• Read up on ES6. Use Arrow-functions, const/let variables, etc. It's not just syntactic sugar, but also more consistent overall.

• Destructure package variables and arrays. It shortens your code and makes it easier to read.

• Use functions. Functions are your friends. Get used to breaking down problems into small pieces and code into small functions. As a rule of thumb no function should have more that two indentations. If you need to nest many if-else clauses, then function them.

• Use semicolons. Even though JS accepts code without semicolons, it's a good practice to use them because it makes the code more readable and less error-prone.

Now, getting back to your code... I made a small refactorization so you can see how it should look, but it needs a complete refactor of architecture as well as code because imo there are many anti-patterns and wrong responsibility assignation, i.e. validations should be frontend, the constants should be in the same file they are used, etc. Nonetheless, this last is more opinion-based.

'use strict';
const User = require('../../models/user'),
 { ERROR_MESSAGES, RESPONSE_STATUS } = require('../../models/constant'),
 helper = require('../../helpers/helper'),
 validator = require('validator');
const register = (req, res) => {
 const email = req.body.email;
 const password = req.body.password;
 const errors = [];
 // input validation
 validate(email, password);
 if (errors.length > 0) {
 return res.json(helper.generateResponse(errors, null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 User.findOne({email: email})
 .then(user => {
 if (user) {
 res.json(helper.generateResponse([ERROR_MESSAGES.DUPLICATE.EMAIL], null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 createUser(email, password);
 }
 })
 .catch(err => {
 res.json(helper.generateResponse([ERROR_MESSAGES.UNKNOW], null, RESPONSE_STATUS.REQUEST_DENIED)) 
 });
 }
}
const validate = (email, password) => {
 if (!email || email.trim().length == 0) {
 errors.push(ERROR_MESSAGES.EMPTY.EMAIL)
 } else if (!validator.isEmail(email)) {
 errors.push(ERROR_MESSAGES.INVALID.EMAIL)
 }
 if (!password) {
 errors.push(ERROR_MESSAGES.EMPTY.PASSWORD)
 }
};
const createUser = (email, password) => {
 let user = new User()
 user.email = email
 user.password = user.generateHash(password)
 const errorValidation = user.validateSync();
 if (errorValidation) {
 errors = helper.getMongooseValidationErrors(errorValidation.errors)
 res.json(helper.generateResponse(errors, null, RESPONSE_STATUS.REQUEST_DENIED))
 } else {
 saveUser(user);
 }
};
const saveUser = user => {
 user.save()
 .then(newUser => res.json(helper.generateResponse(errors, newUser, RESPONSE_STATUS.SUCCESS)))
 .catch(error => res.json(helper.generateResponse([error], null, RESPONSE_STATUS.REQUEST_DENIED)));
};
module.exports = { register };

As I said earlier, I'd suggest a complete refactor not just making syntactic changes like the ones I commented, but also architecture changes. If you need to decide how-to structure the project, etc. Ask other questions in Stackoverflow, maybe :)