HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Check if variable is sent

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkvariablesent

Problem

I have this piece of code in my Node.js API and I feel like it could (should) be optimized.

var apiRouter = express.Router();

apiRouter.route('/user/:username')
.put(function(req, res) {
    User.findOne({
        username: req.params.username
    }).exec(function(err, user) {
    if (err) {
        res.send(err);
    }
    if (!user) {
        res.json({
            success: false,
            message: 'User not found'
        });
    } else {
        // This is the part to optimize
        if (req.body.firstname) {
            user.firstname = req.body.firstname;
        }
        if (req.body.lastname) {
            user.lastname = req.body.lastname;
        }
        if (req.body.email) {
            user.email = req.body.email;
        }
        if (req.body.username) {
            user.username = req.body.username;
        }
        if (req.body.password) {
            // no worries about this, I hash the password before it is saved
            user.password = req.body.password;
        }

        user.save(function(err) {
            if (err) {
                res.send(err);
            }
            res.json({
                success: true,
                message: 'User updated'
            });
        });
    }
});


I use this to update my user. In the future I could (will) have way more parameters than these.

Is there a scalable solution?

I'm thinking about something like this:

var form = req.body;
user.firstname = form.firstname ? form.firstname : user.firstname;
user.lastname = form.lastname ? form.lastname : user.lastname;
// ...


Problem is that I'm not sure it is more scalable. I feel like it is just another way to write if statements.

Solution

I do not believe the current answer solves your optimization need, I believe the correct answer is to usefindOneAndUpdate. This will cut down on the steps need to achieve this same goal. The update function will handle all of the if checks, it will only update fields supplied in the req.body.

User.findOneAndUpdate({ username: req.params.username }, req.body, function (error, User) {
      if (User) {
        res.json({
            success: true,
            message: 'User updated'
        });
      } else {
        // ALWAYS RETURN res.send please
        return res.send(err);
      }
    });


http://mongoosejs.com/docs/api.html#query_Query-findOneAndUpdate for additional docs.

WARNING If res.body has properties that are set as undefined, it will overwrite existing values. You should validate inputs coming in to ensure this is not the case.

Code Snippets

User.findOneAndUpdate({ username: req.params.username }, req.body, function (error, User) {
      if (User) {
        res.json({
            success: true,
            message: 'User updated'
        });
      } else {
        // ALWAYS RETURN res.send please
        return res.send(err);
      }
    });

Context

StackExchange Code Review Q#86732, answer score: 5

Revisions (0)

No revisions yet.