Reputation: 804
Please give me suggestion on how I can optimize my code that is present in custom module.
Below is my module code that you can see and suggest.
var employee = {
all: function (req, res) {
jwt.verify(req.token, 'novaturesol', (err) => {
if (err) {
res.status(400).send("Forbidden or tokken is expired!");
} else {
// database query.
con.query("select * from employees limit 50", function (err, employees) {
if (err) throw err;
// console.log("Result: " + employees);
res.status(200).json(employees);
});
}
});
},
create: function (req, res) {
jwt.verify(req.token, 'novaturesol', (err) => {
if (err) {
res.status(400).send("Forbidden or tokken is expired!");
} else {
// validation array send in response.
const errors = validationResult(req);
if (!errors.isEmpty()) {
return res.status(422).json({
errors: errors.array()
});
}
// random employee number.
let employee_no = Math.floor(Math.random() * Math.floor(9000));
// simple insert query.
let sql = "INSERT INTO employees(emp_no, first_name, last_name, gender, birth_date, hire_date) VALUES('" + employee_no + "','" + req.body.first_name + "','" + req.body.last_name + "','" + req.body.gender + "','" + req.body.birth_date + "','" + req.body.hire_date + "')";
con.query(sql, function (err, result) {
if (err) throw err;
console.log('Record inserted Successfully!');
});
// send response with last inserted employee id.
res.status(200).send({
message: "Successfully added employee!",
last_employee_no: employee_no
});
}
});
},
delete: function (req, res) {
jwt.verify(req.token, 'novaturesol', (err) => {
if (err) {
res.status(400).send("Forbidden or tokken is expired!");
} else {
if (!req.body.employee_no) {
res.status(400).send({
message: "employee_no is required."
});
} else if (isNaN(req.body.employee_no)) {
res.status(400).send({
message: "employee_no must be a integer."
});
} else {
let employee_no = req.body.employee_no;
// delete record.
con.query("DELETE FROM employees where emp_no = '" + employee_no + "'")
res.status(200).send({
message: "Successfully deleted employee",
deleted_employee_no: employee_no
});
}
}
});
},
update: function (req, res) {
jwt.verify(req.token, 'novaturesol', (err) => {
if (err) {
res.status(400).send("Forbidden or tokken is expired!");
} else {
if (!req.body.employee_no) {
res.status(400).send({
message: "employee_no is required."
});
} else if (isNaN(req.body.employee_no)) {
res.status(400).send({
message: "employee_no must be a number."
})
} else if (!req.body.first_name) {
res.status(400).send({
message: "first_name is required."
});
} else if (!req.body.last_name) {
res.status(400).send({
message: "last_name is required."
});
} else if (!req.body.hire_date) {
res.status(400).send({
message: "hire_date is required."
});
} else if (!req.body.birth_date) {
res.status(400).send({
message: "birth_date is required."
});
} else if (!req.body.gender) {
res.status(400).send({
message: "gender is required."
});
} else {
let employee_no = req.body.employee_no;
let first_name = req.body.first_name;
let last_name = req.body.last_name;
let gender = req.body.gender;
let hire_date = req.body.hire_date;
let birth_date = req.body.birth_date;
let sql = "UPDATE employees set first_name = '" + first_name + "' , last_name = '" + last_name + "', gender = '" + gender + "', hire_date = '" + hire_date + "', birth_date = '" + birth_date + "' WHERE emp_no = '" + employee_no + "'";
console.log('the query ' + sql);
con.query(sql, function (err) {
if (err) throw err;
})
res.status(200).send({
message: "Successfuly updated employee record.",
updated_employee_no: employee_no
});
}
}
});
}
};
module.exports = employee;
In each function I need to add jwt.verify for verification? or is there any alternative way to do it?
About Db Queries We write queries in node express like I have did? or is there any other proper way to do that?
Upvotes: 1
Views: 37
Reputation: 2610
You could create a verify middleware to run before each request. That would save you code repetitions.
This code is not tested*
//set verification middleware
function verifyJwt(req,res,nex) {
jwt.verify(req.token, 'novaturesol', (err) => {
err ? res.locals.verified = false : res.locals.verified = true;
next();
})
}
// use before routes
app.use(verifyJwt)
// check in your modulee functions
var employee = {
all: function (req, res) {
if (res.locals.varified) {
// database query.
con.query("select * from employees limit 50", function (err, employees) {
if (err) throw err;
// console.log("Result: " + employees);
res.status(200).json(employees);
});
}
},...
}
Upvotes: 0
Reputation: 303
This code is not maintainable. you should create repositories for your database queries that it will return the data you need for each section. And for the authuntication, you should create middleware in express to handle that before getting in your employee controller. You should not repeat those lines of checking every time.
This a sample for the repositories: areaRepository
And for controller: userController
Authentication and other middlewares: middlewares
Please keep it simple and separated for each section. Maybe my code was a little bit confusing. I hope it will help
Upvotes: 1