1
\$\begingroup\$

I am creating a small Node.js application, but I think the file becomes unreadable. I am certain the file can be split into some better managable file.

Here is my app.js

var sio          = require('socket.io'),
    fs           = require('fs'),
    express      = require('express'),
    connect      = require('connect'),
    MemoryStore  = express.session.MemoryStore,
    app          = express.createServer('http://mysite.dev'),
    sessionStore = new MemoryStore(),
    parseCookie  = connect.utils.parseCookie,
    Session      = connect.middleware.session.Session
    db           = require('mongoose').connect('mongodb://localhost/dev_db'),
    Common       = {
        users: require('./server/models/user').User
    };

app.configure(function () {
    app.use(express.cookieParser());
    app.use(express.session({
        store: sessionStore,
        secret: 'secret',
        key: 'express.sid'
    }));
    app.get('*', function (req, res) {
        res.sendfile(__dirname + '/assets/tmpl/index.html');
    });
});

app.listen(8008);
var io = sio.listen(app);

io.configure(function () {
    io.set('transports', ['websocket']);
});
io.set('authorization', function (data, accept) {
    if (data.headers.cookie) {
        data.cookie = parseCookie(data.headers.cookie);
        data.sessionID = data.cookie['express.sid'];
        data.sessionStore = sessionStore;
        sessionStore.get(data.sessionID, function (err, session) {
            if (err || !session) {
                accept('Error', false);
            } else {
                data.session = new Session(data, session);
                accept(null, true);
            }
        });
    } else {
        return accept('No cookie transmitted.', false);
    }
});

var create = function (socket, signature) {
    var e = event('create', signature), data = [];
    socket.emit(e, {id : 1});
};

var read = function (socket, signature) {
    var e = event('read', signature);
    if(signature.endPoint){
        Common[signature.endPoint][signature.cb](socket, signature.ctx, function(err, data){
            if(err) throw err;
            socket.emit(e, data);
        })
    }

};

var update = function (socket, signature) {
    var e = event('update', signature), data = [];
    socket.emit(e, {success : true});
};

var destroy = function (socket, signature) {
    var e = event('delete', signature), data = [];
    socket.emit(e, {success : true});
};

// creates the event to push to listening clients
var event = function (operation, sig) {
    var e = operation + ':';
    e += sig.endPoint;
    if (sig.cb) e += (':' + sig.cb);
    if (sig.ctx) e += (':' + sig.ctx);

    return e;
};

io.sockets.on('connection', function (socket) {

    var hs = socket.handshake;
    console.log('A socket with sessionID ' + hs.sessionID + ' connected!');
    // setup an inteval that will keep our session fresh
    var intervalID = setInterval(function () {
        hs.session.reload( function () {
            hs.session.touch().save();
        });
    }, 60 * 1000);

    socket.on('init', function(data){
        if(hs.session.userdata){
            fs.readFile(__dirname + '/assets/tmpl/user/dashboard.html', 'utf8', function(err, html){
                if(err) throw err;
                socket.emit('init', {callback: 'dashboard', tmpl: html});
            });
        }else{
            fs.readFile(__dirname + '/assets/tmpl/user/login.html', 'utf8', function(err, html){
                if(err) throw err;
                socket.emit('init', {callback: 'login', tmpl: html});
            });
        }
    });

    socket.on('create', function (data) {
        create(socket, data.signature);
    });
    socket.on('read', function (data) {
        console.log(hs.session);
        read(socket, data.signature);
    });
    socket.on('update', function (data) {
        update(socket, data.signature);
    });
    socket.on('delete', function (data) {
        destroy(socket, data.signature);
    });

    socket.on('disconnect', function () {
        console.log('A socket with sessionID ' + hs.sessionID + ' disconnected!');
        clearInterval(intervalID);
    });
});
\$\endgroup\$
3
\$\begingroup\$

Instead of writing comments, I would avoid using anonymous functions or meaningless names.

For instance,

var create = function (socket, signature) {
    var e = event('create', signature), data = [];
    socket.emit(e, {id : 1});
};

What do you create ? Looking at the code you create an event and then you emit it.

var createAndEmitEvent = function (socket, signature) {
    var e = event('create', signature), data = [];
    socket.emit(e, {id : 1});
};

Here you know by its name. Therefor you don't need a comment block. This block code block doesn't respect the single responsibility principle but principles are meant to be broken. A 2 lines function doesn't need to be splitted.

I would also allow some duplication in the signature.

var createAndEmitEvent = function createAndEmitEvent (socket, signature) { 

The purpose of this is to have the name of the function in the stack trace in case of an error. Otherwise, the function will be identified as anonymous.

Here's another example.

socket.on('init', function(data){
    if(hs.session.userdata){
        fs.readFile(__dirname + '/assets/tmpl/user/dashboard.html', 'utf8', function(err, html){
            if(err) throw err;
            socket.emit('init', {callback: 'dashboard', tmpl: html});
        });
    }else{
        fs.readFile(__dirname + '/assets/tmpl/user/login.html', 'utf8', function(err, html){
            if(err) throw err;
            socket.emit('init', {callback: 'login', tmpl: html});
        });
    }
});

You could create a function called onInitSocket. You also have a another callback inside this function to handle the readFile callback. This is a smell. You could handle this case using promise or using the async module. I recommend the latest in your case in order to not change the paradigm. You also have a variable called hs. If you split your code in functions and later in different file, it won't be so obvious that this name refers to handshake.

Overall, make sure your code speak for itself. You should only have to use comments when the code needs to be unclear (algorithme optimization for instance).

\$\endgroup\$
1
\$\begingroup\$

Before trying to split the program into separate files, I would suggest first commenting your functions to explain what they are accomplishing in the context of your application. Then, add comments that group things under a specific "section", e.g.:

    /* --- INITIALIZE ALL GLOBAL DATA FIRST --- */

    var initializeFirstDataSet = function () { ... }();
    var initializeSecondDataSet = function () { ... }();

    /* --- VALIDATE USER INPUT --- */
    /*
     * Our application is especially sensitive to erroneous user input;
     * therefore, we will provide additional overhead to user input validation
     * and stop all further processing if any input is invalid.
     */
    /**
     * we provide our users the flexibility of giving email addresses or phone numbers
     * as user names. ensure they have entered in one of those here.
     * NOTE: we will not test whether the user name itself exists here, only that
     *       it's possible that the input could be a user name.
     * TODO[me]: our application may be using a more systematic validation
     *           from other parts of our system. include that here via another
     *           file if available. 
     * @param userName String - email address or phone number
     * @return Boolean - whether user name is of valid format
     */
    var validateUserName = function (userName) {
        // do regex-matching and other necessary validation here

    }( /* input GET/POST parameter here */ );

    /* --- WIRE LIFE-CYCLE EVENTS --- */
    /*
     * our application has a number of events that are part of the normal
     * "lifecycle" of a REST API. include the handlers here.
     * TODO[me]: is there 
     */
    var create = function () { ... }();

As I spent time commenting the various sections of my code and trying to group them, I realized that some things may be retrieved from other files, and some things may be moved to another file for more streamlining.

At the very least, once you start commenting, your code becomes more understandable to someone who has not seen the code before.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Well, the script I wrote is pretty self-explanatory. In the first section I include all that is needed for the app, after that I init the server, after that init session/cookie, lastly init socket.io. I thought it would be understandable to any Node programmer. The main problem is not that I do not know what to split, but more how to split the file. What kind of syntax I would use. \$\endgroup\$ – Saif Bechan Apr 2 '12 at 0:31
  • \$\begingroup\$ When you try to split up code, usually you need to first figure out what code can be grouped together in logical units of work. Then deciding whether to put them in different files will require engineering trade-off mentality, where you ask questions, such as, "is this code expected to be isolated to this app as a one-off, or will this more streamlined for a larger web-app?" Asking yourself these questions is helpful to prevent segmenting your code too much and making debugging harder. Also, a key issue in readability is not whether someone understands the syntax (continued...) \$\endgroup\$ – Ryan Apr 2 '12 at 3:42
  • \$\begingroup\$ of the framework you've chosen. It's more important to explain the rationale of your code structure in the context of what you are trying to achieve with your individual app. I've seen a million sql queries, for example, but it makes it much easier as an engineer to understand why this particular sql query is being made for my particular app. When you describe the rationale of your app as you add functionality, it also makes it easier to understand where natural code grouping is occurring for your app. \$\endgroup\$ – Ryan Apr 2 '12 at 3:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.