0

In order to separate the business logic and data access layer , I have separated the files and folders in a non-event driven way. Here is an example of the User signup process I have taken , Here is my Business logic layer file.

var userDA = require('./../DataAccess/UserDA');
module.exports = {
  signUpUser: function(userGiven)
  {
    //Some bookeeping
    userGiven.userType = "admin";

    return userDA.save(tutorGiven);
   }
}

and here is my data access file

"use strict";
var mongoose = require('mongoose');
if(!mongoose.connection)
    mongoose.connect('mongodb://localhost/test');
var User = require('./../models/User');

module.exports ={

    save : function(UserGiven){
    var pass = true;

    var user = new User(UserGiven);
    user.save(function (err) {
        if(err) {
            console.log(err);
            pass = false;
        }
    });
    return pass;
},

getUser: function (email) {
    var user = null;

    user.findOne({email:email},function(err,foundUser){
        if(err)
            console.log(err);
        else
            user = foundUser;
    });
    return user;

   }
}

This a for a real project of medium-large size , and as a newcomer in nodejs I was wondering and in need of an expert advice if It would be any problem if I take this kind of designing approach?

Tamim Addari
  • 7,591
  • 9
  • 40
  • 59
  • Well, your `getUser` function will always return `null` as the callback passed to `findOne` won't be called until _control returns to the event loop_ (when the current turn ends) which will at least be after the function returns. – Dan D. Feb 11 '16 at 16:08

1 Answers1

1

You are running into a classic pitfall of those new to asynchronous programming in node. For example, consider this code:

setTimeout(function () {
    console.log('timer');
}, 200);
console.log('done');

which will output the following:

done
timer

The reason for this is that setTimeout is scheduling asynchronous work for later. What actually happens is the setTimeout function returns immediately, console.log('done') is called, then ~200 ms later the callback function is called and console.log('timer') is called.

A more proper approach would be to use callbacks to indicate when your work is complete. Something like this:

"use strict";
var mongoose = require('mongoose');
if(!mongoose.connection)
    mongoose.connect('mongodb://localhost/test');
var User = require('./../models/User');

module.exports ={

    save : function(UserGiven, callback){
        var pass = true;
        var user = new User(UserGiven);
        user.save(function (err) {
            if(err) {
                console.log(err);
                return callback(err);
            }
            return callback();
        });
    },

    getUser: function (email, callback) {

        user.findOne({email:email},function(err,foundUser){
            if(err) {
                console.log(err);
                return callback(err);
            }
            return callback(null, foundUser);
        });
    }
}

This follows the convention of error-first callbacks, where a callback's first parameter is an error, the second is the data. Your first code segment would then look like this:

var userDA = require('./../DataAccess/UserDA');
module.exports = {
    signUpUser: function(userGiven, callback) {
        //Some bookeeping
        userGiven.userType = "admin";
        userDA.save(tutorGiven, callback);
    }
}

Now, you would call signUpUser like so:

var userRepo = require('<PATH TO FILE>');
var someUser = {};
userRepo.signUpUser(someUser, function(err, user) {
    if (err) {
        // oops!
    }

    // do whatever you need to accomplish after the user is signed up
});
Kevin Burdett
  • 2,892
  • 1
  • 12
  • 19
  • is it okay to separate like this ? or should I just directly use mongoose from the BLL ? I have some problem testing in environment like this. – Tamim Addari Feb 12 '16 at 06:47
  • It is certainly okay, but whether or not it is best would be very hard for me to say without knowing your system and your requirements. It's also heavily opinionated. I believe it is that it is a worthwhile abstraction. It allows you more freedom to replace your database and gives you a location to introduce dependency injection, which can in turn enable unit testing. I hope that is helpful – Kevin Burdett Feb 12 '16 at 13:38