CODEWITHSUNDEEP
node.jsexpressmoduleredisexport
michelem
michelem

Reputation: 14590

Node.js code hints

Hello I would like to know if I'm right with this kind of code and if there is a better way to do it.

There is a module to get the homepage.

app.js:

[...]
// Get homepage
app.get('/', frontend.index);
[...]

The frontend.index function have to get a couple of data obj from redis (util.getRemoteConfig and util.getGlobalStats) and use them to render the Jade template, so I did this.

frontend.js:

[...]
// [GET] Homepage
exports.index = function(req, res){   
    var remoteConfig = {};
    var globalStats = {};

    util.getRemoteConfig(function(remoteConfig) { 

        var version = 'n.a.';
        if (remoteConfig.version)
            version = remoteConfig.version;

        util.getGlobalStats(function(globalStats) {

            res.render('index.jade', 
                { title: config.items.default_title, version: version, global_stats: JSON.stringify(globalStats) }
            );
        });
    });     
};
[...]

The util module gets the data from redis and pass it via callback.

util.js:

[...]
exports.getRemoteConfig = function(cb) {
    client.hget('remote_config', function(err, obj) {
        if (err) return cb(err);
        // do something with obj and return it
        cb(obj);
    });
};

exports.getGlobalStats = function(cb) {
    client.hgetall("global_stats", function (err, obj) {
        if (err) return cb(err);
        // do something with obj and return it
        cb(obj);
    });
};
[...]

This is working fine but is it really correct? Can I do something better than this?

Thanks any hint will be useful.

Upvotes: 0

Views: 94

Answers (1)

Max
Max

Reputation: 368

If remote config and global stats are route independent, you should probably put them into a middle-ware instead of controller:

middlewares.js:

exports.remoteConfig = function(req,res,next){
    util.getRemoteConfig(function(err,config){
        if(err){
            return next(err);
        }
        req._remote_config = config;
        return next(null);
    });
}

controllers.js:

exports.index = function(req, res){   
    var remoteConfig = req._remote_config;
    var globalStats = req._global_stats;

    res.render('index.jade', { 
        title: config.items.default_title, 
        version: version, 
        global_stats: JSON.stringify(globalStats) 
    };
};

app.js:

app.get('/', middleware.remoteConfig, middleware.globalStats,controllers.index);

or

app.use(middleware.remoteConfig);
app.use(middleware.globalStats);

if the middleware should be used for all routes.

Note: the code is untested.

Upvotes: 1

Related Questions