CODEWITHSUNDEEP
node.jsexpress
That1guyoverthr
That1guyoverthr

Reputation: 1131

Express.js Best Practices

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!

Upvotes: 0

Views: 910

Answers (2)

CleverPatrick
CleverPatrick

Reputation: 9483

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.

Upvotes: 0

OldGeeksGuide
OldGeeksGuide

Reputation: 2918

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'));
};

Upvotes: 1

Related Questions