Skip to main content
deleted 3 characters in body; edited title
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

How to improve Express.js controller with Node.js style exports.methods

So I have some Node/Express.js controllers which implement the exports.method Node module convention. This particular controller also has a few helper methods. The code works fine!

How to improve Express.js controller with Node.js style exports.methods

So I have some Node/Express.js controllers which implement the exports.method Node module convention. This particular controller also has a few helper methods. The code works fine!

Express.js controller with Node.js style exports.methods

I have some Node/Express.js controllers which implement the exports.method Node module convention. This particular controller also has a few helper methods. The code works fine!

Tweeted twitter.com/#!/StackCodeReview/status/431963963367387136
Source Link
Jon Miles
  • 161
  • 3
  • 7

How to improve Express.js controller with Node.js style exports.methods

So I have some Node/Express.js controllers which implement the exports.method Node module convention. This particular controller also has a few helper methods. The code works fine!

However, I'm a stickler for conventions, best practices etc. and recently I've been going back over one of my project with jshint (Sublime Text 3) and noticed my controllers throw up lots of complaints. Well really it's the same complaint over and over again - that methods called inside a exports closure are not defined, they are in fact defined in the controller but outside of the exports closure so they can be reused.

Example, jshint complaint

  • 9:14 'parseQueryString' is not defined

So really what I'm asking, is how do you recommend I structure my controller (and also more generally node modules that use exports closures) for correctness / best practice?

Here's the code for your reference:

/* global require, exports */
'use strict';

var mongoose = require('mongoose'),
    Time = mongoose.model('Time');

exports.findAll = function (req, res) {

    var qs = parseQueryString(req);

    // Importantly we should only query times for current user.
    qs.query.userId = getUserVal(req, '_id');

    log(req, 'Retrieving all times');
    Time.find(qs.query, qs.fields, qs.options, function (err, result) {
        if (err) { return handleError(req, res, err); }
        res.send(result);
    });
};

exports.findById = function (req, res) {

    var id = req.params.id,
        userId = req.user.get('_id');

    log(req, 'Retrieving time: ' + id);
    Time.findOne({'_id':id,'userId':userId}, function (err, result) {
        if (err) { return handleError(req, res, err); }
        res.send(result);
    });
};

exports.addTime = function (req, res) {

    var time = new Time(req.body);
    
    // Enforce user association
    time.userId = req.user.get('_id');
    time.user = req.user.get('name');

    log(req, 'Adding time: ' + time);
    time.save(function (err) {
        if (err) { return handleError(req, res, err); }
        Time.findById(time._id, function (err, found) {
            time = found;
        });
        res.send(time);
    });
};

exports.updateTime = function (req, res) {

    var id = req.params.id,
        userId = req.user.get('_id'),
        time = req.body;
    delete time._id;

    log(req, 'Updating time: ' + id);
    Time.update({'_id':id,'userId':userId}, time, function (err, rowsAffected, raw) {
        if (err) { return handleError(req, res, err); }
        console.log('The number of updated documents was %d', rowsAffected);
        console.log('The raw response from Mongo was ', raw);
    });
};

exports.deleteTime = function (req, res) {
    
    var id = req.params.id,
        userId = req.user.get('_id');

    log(req, 'Deleting time: ' + id);
    Time.remove({'_id':id,'userId':userId}, function (err) {
        if (err) { return handleError(req, res, err); }
        res.send(req.body);
    });
};


// Helpers

handleError = function(req, res, err) {
    log(req, 'error : ' + err);
    res.send({'error' : err});
};

log = function(req, msg) {
    console.log(msg + ' | u:' + getUserVal(req, '_id'));
};

getUserVal = function(req, val) {
    return req.user ? req.user.get(val) : '';
};

// Extract options from query string 
// and format for mongoose query as follows:
// query = {'client':'ABK','category':'Email'}
// fields = 'client category'
// options = {limit:10,skip:0}
parseQueryString = function(req) {
    
    var result = {
        query: JSON.parse(JSON.stringify(req.query)),
        fields: '',
        options: {}
    },
    tmp = result.query;

    for (var key in tmp) {
        if (tmp.hasOwnProperty(key)) {
            var val = tmp[key];
            // console.log(' + key + ' = ' + val);
            if (key === ':show') {
                result.fields = val.replace(',', ' ');
                delete result.query[key];
            }
            else if (key === ':limit') {
                result.options.limit = parseInt(val);
                delete result.query[key];
            }
            else if (key === ':skip') {
                result.options.skip = parseInt(val);
                delete result.query[key];
            }
        }
    }
    // console.log('result = ' + JSON.stringify(result));
    return result;
};