HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

node.js + Express code critique

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codenodeexpresscritique

Problem

I'm learning Node. I'm working on a personal project that uses Node + Express + Node-MySQL + Generic-Pool (for MySQL pooling). I am new to the idea of module.exports and I'm still grokking it.

The following code works. However I want to organize my code in a way that is logical and optimal, and MVCish. I am wondering if those of you who have done this before could take a look at the following and let me know if this is the "right" way to structure things. (Yes I know this going to depend a lot on the developer. But at least knowing if there are obvious problems in my approach would be helpful.)

My folder structure looks roughly like this:

app.js
routes/
models/
base/


In my main app.js file I have...

var express = require( "express" ),
    mysql = require( "mysql" ),
    generic_pool = require ( "generic-pool" ),
    routes = require( "./routes" );

var app = module.exports = express(),
    pool = generic_pool.Pool({
        name: "mysql",
        create: function( cb ) {
            var db = mysql.createConnection( require( "./config/db" ) );
            db.connect();
            cb( null, db );
        },
        destroy: function( db ) {
            db.end();
        },
    });

app.configure(function(){
    app.set( "pool", pool );
    ...
});
routes( app );
app.listen(80, function());


Here is an example route/ file "stuff.js"

module.exports = function( app ){
    var model = require( "../models/stuff" );
    model.init( app );
    app.get( "/stuff", function( req, res ){
        model.get( function( result ){
            res.json( result );
        });
    });
};


Here is an example model/ file "stuff.js"

var db;
module.exports.init = function( app ){
    db = require( "../base/db" );
    db.init( app );
};
module.exports.get = function( cb ){
    var sql = "SELECT * FROM stuff";
    db.do( sql, false, cb );
};


And here is the base/db file

```
var pool;
module.exports.init = function( app ){
pool = app.get( "p

Solution

Your code looks fine.

However, you are not showing us much to review, definitely come back when there is more to review.

Some minor comments:

  • Having your db connection config through ./config/db is good



  • You probably want to throw an exception if ( typeof cb !== "function" ) instead of silently ignoring that



  • The base/db code nests 4 levels of callback, you should look into queue management libraries for node.js

Context

StackExchange Code Review Q#21202, answer score: 3

Revisions (0)

No revisions yet.