patternjavascriptMinor
node.js + Express code critique
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:
In my main app.js file I have...
Here is an example route/ file "stuff.js"
Here is an example model/ file "stuff.js"
And here is the base/db file
```
var pool;
module.exports.init = function( app ){
pool = app.get( "p
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:
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.