patternjavascriptMinor
Node exports, architecture
Viewed 0 times
exportsnodearchitecture
Problem
My project is written in node JS + Express and has the following architecture:
Models objects only doing SQL queries like this:
and many many functions connected with the user in that user.js module.
The only thing I can do with that architecture is to place functions to the different module.
I'd appreciate any critiques of that architecture. How can I do better?
- routes (e.g controllers)
- models (stateless functional modules)
- app
Models objects only doing SQL queries like this:
exports.setPassword = function (id, password, callback) {
pg.connect(conString, function (err, client, done) {
if (err) callback(err);
else {
client.query('UPDATE users SET password=$1 WHERE id=$2', [password, id], function (err) {
done();
if (err) callback(err);
else callback(null);
});
}
});
};
exports.setStatus = function (id, status, callback) {
pg.connect(conString, function (err, client, done) {
if (err) callback(err);
else {
client.query('UPDATE users SET status=$1 WHERE id=$2', [status, id], function (err) {
done();
if (err) callback(err);
else callback(null);
});
}
});
};
exports.getStayInTouch = function (userId, callback) {
pg.connect(conString, function (err, client, done) {
if (err) callback(err);
else {
client.query('SELECT * FROM stay_in_touch_contacts WHERE user_id=$1', [userId], function (err, results) {
done();
if (err) callback(err);
else callback(null, results.rows[0]);
});
}
});
};and many many functions connected with the user in that user.js module.
The only thing I can do with that architecture is to place functions to the different module.
I'd appreciate any critiques of that architecture. How can I do better?
Solution
That does not look very DRY ( Dont repeat yourself ), not to mention that it does not look good that every function must be aware of the connection string.
For updates, you could do something like this:
I added curly braces and newlines to your
EDIT: some thoughts on selects, you probably want a few generic selection functions, if you create one that is meant for selecting on an id, then you can get away with always returning the first data record if any.
For updates, you could do something like this:
exports.setPassword = function (id, password, callback) {
genericUpdate('UPDATE users SET password=$1 WHERE id=$2', [password, id], callback );
};
exports.setStatus = function (id, status, callback) {
genericUpdate('UPDATE users SET status=$1 WHERE id=$2', [status, id], callback );
};
function genericUpdate( queryString, parameters, callback )
{
callback = callback || function(){};
pg.connect(conString, function (err, client, done) {
if (err){
callback(err);
} else {
client.query(queryString, parameters, function (err) {
done();
callback(err || null);
});
}
});
}I added curly braces and newlines to your
if statements, other than that your code seems fine.EDIT: some thoughts on selects, you probably want a few generic selection functions, if you create one that is meant for selecting on an id, then you can get away with always returning the first data record if any.
exports.getStayInTouch = function (userId, callback) {
selectById( 'SELECT * FROM stay_in_touch_contacts WHERE user_id=$1' , userId , callback );
};
function selectById( queryString, id, callback )
{
callback = callback || function(){};
pg.connect(conString, function (err, client, done) {
if (err){
callback(err);
} else {
client.query(queryString, id, function (err,results) {
done();
if(err)
callback(err)
else
callback(null, results.rows[0] );
});
}
});
}Code Snippets
exports.setPassword = function (id, password, callback) {
genericUpdate('UPDATE users SET password=$1 WHERE id=$2', [password, id], callback );
};
exports.setStatus = function (id, status, callback) {
genericUpdate('UPDATE users SET status=$1 WHERE id=$2', [status, id], callback );
};
function genericUpdate( queryString, parameters, callback )
{
callback = callback || function(){};
pg.connect(conString, function (err, client, done) {
if (err){
callback(err);
} else {
client.query(queryString, parameters, function (err) {
done();
callback(err || null);
});
}
});
}exports.getStayInTouch = function (userId, callback) {
selectById( 'SELECT * FROM stay_in_touch_contacts WHERE user_id=$1' , userId , callback );
};
function selectById( queryString, id, callback )
{
callback = callback || function(){};
pg.connect(conString, function (err, client, done) {
if (err){
callback(err);
} else {
client.query(queryString, id, function (err,results) {
done();
if(err)
callback(err)
else
callback(null, results.rows[0] );
});
}
});
}Context
StackExchange Code Review Q#44744, answer score: 4
Revisions (0)
No revisions yet.