Express.js handler to update user fields

Question

I have an express handler which i thought was pretty simple, but CodeClimate flagged this method as having a Cognitive complexity of 6 (5 is the max by default without flagging something)

Curious how I could simplify this method to reduce Cognitive complexity.

function updateUser (req, res) {
 // Update any fields that were passed in.
 // Explicitly checking for undefined b/c passing null values should set them to null in the db
 if (req.body.firstName !== undefined) {
 req.user.firstName = req.body.firstName
 }
 if (req.body.lastName !== undefined) {
 req.user.lastName = req.body.lastName
 }
 if (req.body.email !== undefined) {
 req.user.email = req.body.email
 }
 if (req.body.phone !== undefined) {
 req.user.phone = req.body.phone
 }
 if (req.body.fax !== undefined) {
 req.user.fax = req.body.fax
 }
 if (req.body.notes !== undefined) {
 req.user.notes = req.body.notes
 }
 req.user.save()
 .then(user => {
 return res.status(HttpStatus.OK).send(user)
 })
 .catch(err => {
 req.log.error(err)
 return handleErr(res, HttpStatus.INTERNAL_SERVER_ERROR, err.message)
 })
}

• Alexei: Cognitive complexity or cyclomatic complexity (as in the added tag)?

Answer 1

Assignments can be contracted to something like below:

 req.user.firstName = req.body.firstName || req.user.firstName;
 req.user.lastName = req.body.lastName || req.user.lastName;
 req.user.email = req.body.email || req.user.email;
 req.user.phone = req.body.phone || req.user.phone;
 req.user.fax = req.body.fax || req.user.fax;
 req.user.notes = req.body.notes || req.user.notes;

I think it can further be reduced if properties are put in some array and iterated over (since source and destination property have the same name), but I would stick with this arguably more readable text.

I am not familiar with express.js, so the following might not be applicable (I usually work with Typescript in Angular).

Your request have both a body and some user property. It is recommended to clearly separate output from input in order to obtain pure functions as much as possible. Something like below:

 updateUserFromRequest(req, user) {
 // clone to avoid changing provided reference
 var ret = [... user];
 // assignment logic here using ret
 return ret;
 }

• Catfish: This is a bit off. I only want to set req.user.xxx if it exists on req.body (meaning if it's part of the request payload)

Answer 2

If you are using Mongoose, another possibility is to simply use your model validation to check if required data are present or not.

Then it will give you something like this:

function updateUser (req, res) {
 req.user.set(req.body);
 req.user.save()
 //... Then catch and handle validation error :)
}

More info on .set() method.
More info on model validation.

• Catfish: Not using mongoose. Using Sequelize. It doesn't validate payloads though.
• laudeon: You can see more about model validation with sequeliez here : docs.sequelizejs.com/manual/tutorial/… but it seems to mee that it works the same :)