6
\$\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');
 }
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 18, 2019 at 20:31
\$\endgroup\$
1
  • 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\$ Commented Nov 19, 2019 at 0:06

1 Answer 1

4
\$\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.

answered Nov 18, 2019 at 23:46
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.