Wednesday, September 2, 2015

Refactoring Express Router Middlewares

This past weekend I went back to an old comic to fix some bad code that it used. It had been bothering me for a long time. I knew it was not great when I made it, but since it was a bit of a rush-job (to capitalize on the sudden popularity of a dress color meme) I just lived with it. But no more! It had to be cleaned-up!

The problem with it was that the comic in question stored data about how people interacted with it, but never persisted that data to disk. Every time I re-started the comic application, the previous data was lost. No problem, I just put in a timer that wrote the data as a JSON string to disk every 30 minutes. When the application starts, it reads the file.

But when I was adding this stuff, it bothered me how all the code for the routes for this comic were scattered about in the main app.js file. They really should have been put in a separate file and configured as a router. Again, no problem. I moved the four routes into a separate router module and included it from app.js.

But then I noticed something else that bothered me. I had a bunch of router middlewares now that all used very repetitious blocks of code. Seemed like it was time to make more improvements.

The code originally looked like this.

//------------ set up routes for /data/*

app.use('/data', dataRoutes({
    express: express,
    auth: ensureAuthenticated,
    dataSource: cfact
}));

//------------ set up routes for /memeGen/*

app.use('/memeGen', memeRoutes({
    express: express,
    config: conf
}));

//------------ set up routes for /fb/*

app.use('/fb', fbRoutes({
    express: express,
    config: conf
}));

//------------ set up routes for /colors/*

app.use('/colors', colorRoutes({
    express: express
}));

//------------ set up routes for /images/*

app.use('/images', imgRoutes({
    express: express,
    auth: ensureAuthenticated,
    dataSource: cfact
}));

All the middlewares were passed a very similar object into their constructor, but it was not always identical. Some needed only a single item, some needed three. But they all overlapped. I decided to create the options object only once and pass it to all the middlewares. They could then use whatever they needed and ignore what they didn't. But instead of copying-and-pasting a block of code whenever I want to add another one, I created an array of their names and a function to set everything up. Now I have this:

var express = require('express');

var routers = ["data", "images", "memeGen", "colors", "fb"];

var app = express();

// ... other stuff ...

(function setupRouters() {

    var opts = {
            express: express,
            auth: ensureAuthenticated,
            dataSource: cfact,
            config: conf
    };

    routers.forEach(function(val) {

        try {
            console.log("loading router [" + val + "] ...");
            var r = require("./routers/" + val);
            app.use("/" + val, r(opts));
        } catch (e) {
            console.error("error loading router [" + val + "] - " + e);
        }

    });

})();

With the array of router names configured, I create and call a function which goes over that list and creates each router. It sets each one up with the path to match its name. It's now very easy for me to add new router middlewares.

I always enjoy a good refactoring. Much more than I enjoy taking a picture of myself (see today's comic).

Amphibian.com comic for 2 September 2015