Express.js Best Practices

Question

I have defined the following express route(s) in server.js:

app.get('/adfeed', adfeed.findAll);

app.get('/subscriptions', subscriptions.findAll);

app.get('/cronjob/match', cronjob.match);

Function called when performing GET on /adfeed is:

exports.findAll = function(req, res) {
  mongo.Db.connect(mongoUri, function (err, db) {
    db.collection('adfeed', function(er, collection) {
      collection.find().toArray(function(err, items) {
        res.send(items);
        db.close();
      });
    });
  });
}

Function called when performing GET on /subscriptions is:

exports.findAll = function(req, res) {
    console.log("Get All Subscriptions");
  mongo.Db.connect(mongoUri, function (err, db) {
    db.collection('subscriptions', function(err, collection) {
      collection.find().toArray(function(err, items) {
        res.send(items);
        db.close();
      });
    });
 });
}

QUESTION: /cronjob/match needs to use BOTH the above functions. Is it best practice to call an Express route from an Express route? Is there a better way to do this without duplicating code all over the place?

Thanks for help!

You didn't ask this but you shouldn't be opening/closing your mongo connection like that: http://stackoverflow.com/questions/10656574/how-to-manage-mongodb-connections-in-a-nodejs-webapp — dc5, Jul 02 '14 at 01:50
Duplicating code is not always the worst thing in the world, as long as it's not too much and it allows for easier maintenance. But if it makes sense, you can refactor out your logic into separate libraries that the routes call, thereby eliminating the duplication. — dylants, Jul 02 '14 at 13:50

score 1 · Answer 1 · answered Jul 02 '14 at 01:39

You could avoid code duplication using a function that generates the function you want, which is easier than it sounds:

function findStuffFn(typeOfStuff) {
  return function (err, db) {
    db.collection(typeOfStuff, function(err, collection) {
      collection.find().toArray(function(err, items) {
        res.send(items);
        db.close();
      });
    });
  };
}

This is will return a function that is just like your code above, but with a parameter replacing the string. Thus your code could look like this:

exports.findAll = function(req, res) {
  mongo.Db.connect(mongoUri, findStuffFn('adfeed'));
};

and

exports.findAll = function(req, res) {
  console.log("Get All Subscriptions");
  mongo.Db.connect(mongoUri, findStuffFn('subscriptions'));
};

Thanks for this, but my question more pertains to how one would call and express route from an express route. Or if this is wrong.. Any ideas? Am I making sense? — That1guyoverthr, Jul 02 '14 at 03:14

score 0 · Answer 2 · answered Jul 12 '15 at 18:17

This is where the idea of Separation of Concerns comes into play. In this case you want a separate nodejs module which makes the database calls. The module exposes two methods.

/adfeed calls function A /subscriptions calls function B And, /cronjob calls both functions.

By using SoC you are able to increase the reuse of code. Your controller methods are not directly calling db code. They are responsible for only one thing.

Express.js Best Practices

2 Answers2