Router for quering MySQL results in NodeJS

Question

What should I consider in order to refactor the following code ?

It seems very close to proudly become a Pyramid of Doom:

router.get('/:id', function(req, res) {
 getId = req.params.id
 obj = {}
 db.con.query('SELECT * FROM employees where id=?', getId, function(err, results) {
 if (err) {
 console.log('error in query')
 return
 } else {
 try {
 obj = {
 id: results[0].id,
 name: results[0].name,
 location: results[0].location
 }
 res.render('showuser')
 } catch (err) {
 res.status(500)
 res.render('error', { error: err, id: getId })
 }
 }
 })
});

• James: Really doesn't look that bad, regardless consider using async / await for more succinct code. Presumably the DB lib you're using supports Promises.

Answer 1

Are we actually using the obj variable? It seems like you are not using it unless you were intending to pass to the render function. I don't think the try block is necessary. You can first check if results contains elements in it, so doing results[0].X will not return you an error since results[0] will never be undefined.

Here's an improved version:

router.get('/:id', function(req, res) {
 const getId = req.params.id;
 db.con.query('SELECT * FROM employees where id = ?', getId, function(err, results) {
 if (err || !results.length) {
 console.log('error in query');
 res.status(500).render('error', { error: err, id: getId });
 return;
 }
 var obj = {
 id: results[0].id,
 name: results[0].name,
 location: results[0].location
 };
 res.render('showuser', obj); // Assuming you're using obj to render
 });
});

Answer 2

router.get('/:id', function(req, res) {
 const getId = req.params.id
 db.con.query('SELECT * FROM employees where id=?', getId, function(err, results) {
 if (err) {
 console.log('error in query')
 return
 }
 try {
 const obj = {
 id: results[0].id,
 name: results[0].name,
 location: results[0].location
 }
 res.render('showuser')
 } catch (err) {
 res.status(500)
 res.render('error', { error: err, id: getId })
 }
 })
});

There are a few of things I changed

• added const to getId now it may not interfere with other variables called getId in your module
• moved definition of the obj inside the try block, since this is the only place you are using it as well as added const for the same reason
• since you have return in your if (err) condition, the function will end there if it errors. Therefore, else is unnecessary here and was removed