Update user's last login date

Question

This function updates the user last login date on the user collection. I guess there are too many brackets and much spaghetti. How can I shorten this code?

(function () {
 'use strict';
 var database = require('mongodb');
 var util = require('util');
 var constants = require('../constants');
 var dbhelper = {
 updateLastSeenDate: function (me) {
 database.connect(constants.variables.connection, function (err, db) {
 if (err) {
 db.close();
 throw err;
 }
 var bsonID = database.BSONPure.ObjectID(me);
 var collection = db.collection('users');
 collection.findOne({
 _id: bsonID
 }, function (err, result) {
 if (result) {
 result.lastLoginDate = Date.now();
 collection.update({
 _id: bsonID
 }, result, function (err, u) {
 if (err) {
 db.close();
 throw err;
 }
 util.log('Update complete: updateLastSeenDate');
 db.close();
 }); //end of update
 }
 });
 });
 }
 };
 module.exports = dbhelper;
}());

• Marc-Andre: Welcome to Code Review! Your question only contains code. Could you edit it with a proper context ? You could specify what your code is doing, what you think is the major point that need refactoring (note that we could do general review but if you a special point you can mention it).

Answer 1

Here's some points. I commented directly in the code.

(function () {
 'use strict';
 //Why do you use `database` instead of `mongo` or `mongodb`.
 //It can become confusing later in the code when you are using `db`.
 //It will be hard to tell the diffence between database and db.
 var database = require('mongodb');
 var util = require('util');
 var constants = require('../constants');
 var dbhelper = {
 //`me` Will it be you all the time ?
 //You should think a more meaningful name such as `logedInUserId`
 updateLastSeenDate: function (me) {
 //Do you really want to connect and close the database everything you want to update a field ?
 //You should consider keeping the connection alive and share it across the whole process.
 database.connect(constants.variables.connection, function (err, db) {
 if (err) {
 db.close();
 throw err;
 }
 var bsonID = database.BSONPure.ObjectID(me);
 var collection = db.collection('users');
 //nesting calls to async function is affecting readability, testability and maintability.
 //You should consider using workflow library such as async
 collection.findOne({
 _id: bsonID
 }, function (err, result) {
 if (result) {
 result.lastLoginDate = Date.now();
 //Here's another nested call
 collection.update({
 _id: bsonID
 }, result, function (err, u) {
 //Using async you could handle errors in only one place.
 if (err) {
 db.close();
 throw err;
 }
 util.log('Update complete: updateLastSeenDate');
 db.close();
 });
 }
 });
 });
 }
 };
 module.exports = dbhelper;
}());

I would use async https://github.com/caolan/async#waterfall, change a few variable names and share the connection. Since node.js in single threaded you can open only one connection and share it to all your modules.

• Sebastian: how can i do "keeping the connection alive", can you prefer a sample
• jackdbernier: You could check this answer stackoverflow.com/a/14464750/327004 and there's an example here groups.google.com/forum/#!msg/node-mongodb-native/mSGnnuG8C1o/…