5
\$\begingroup\$

I have some questions about best practices to apply in JavaScript. I would like to ask about my code and how I could improve.

Starting with an example, I have 2 functions with different ways of handling promises

Here I have a route to add a user:

 addUsers(req,res,next) {
    try {
        const {name,email,login} = req.body;
        User.existLogin(req.body.login)
        .then(async result => {
            if(!result){
                const password = await bcrypt.hashSync(req.body.password, 10);
                User.create({ 
                    name, email, login, password }).then(result => {
                    res.status(201).json({Results: result.dataValues})
                })
            }else{
                return res.status(409).json({message: 'Login already exists'}); 
            }
        })
    } catch (error) {
        res.status(500).json({error: error})
    }
}

And here I have another function to login:

async login(req,res){
    const  user  = await User.existLogin(req.body.login);
    if (!user) { return res.status(400).json({result: 'Login is wrong '});} 
    const isPassword = await User.isPassword(user.dataValues.password, req.body.password);
    if (!isPassword) { return res.status(400).json({result: 'Password is wrong '}); } 
    const jwt = auth.signjwt(auth.payload(user));
    console.log(jwt);
    res.status(200);
    res.json(jwt);
}

Looking at this the login function seems to me to be cleaner, but I have doubts if really yours I could improve in one of two (I follow different logics to do both).

I have an auth folder, where I export functions, such as my payload, my sign, my middleware to validate my jwt, but I don't know if this is a correct decision.

const jwt = require('jsonwebtoken');
const User = require('../models/User')
const config= require('../config/dbconfig');
const moment = require('moment');

module.exports = {
    signjwt (payload) {
        return jwt.sign(payload, 
            config.secretToken
                   )
    },
    payload (usuario) {
        return {
            sub: usuario.id,
            name: usuario.nome,
            email: usuario.email,
            login: usuario.username,
            admin: true,
            iat: Math.floor(moment.now()/1000), // Timestamp de hoje
            exp: moment().add(10, 'minutes').unix() // Validade de 2 dias
        }
    },
    async auth(req,res,next){
        const token = req.header('Authorization');
        console.log(token);
        if(!token) return res.status(401).json('Unauthorized');
        try{
            const decoded = jwt.verify(token,config.secretToken);
            const user = await User.findByPk(decoded.sub);
            console.log(user);
            if(!user){
                return res.status(404).json('User not Found');
            }
            res.json(user);
            next();
        }catch(error){
            console.error(error);
            res.status(400).json('Invalid Token');
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ "with different ways of handling promises" - avoid mixing await and .then(…) syntax, yes. Don't pass async functions as callbacks. Btw, one mistake that probably stems from this confusion is that you're not handling rejections from User.existLogin. \$\endgroup\$ – Bergi Nov 19 '19 at 0:06
3
\$\begingroup\$

Follow some standards, use code linting (for example ESLint) and code formatting (for example Prettier), try to prevent code duplication as much as possible, use design patterns and don't complex things. Those things will make your code more readable for others.

There is no right or wrong while the code works and it's easy to read.


Martin Fowler.

Any fool can write code that a computer can understand. Good programmers write code that humans can understand.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.