How to improve my code in nodejs?

Question

I am working in MEAN STACK application, I want to improve my following code.

app.js

var express = require("express");
var router = express.Router();
var Comments = require('../models/Comments');
var Posts = require('../models/Posts');

//use in strict mode
'use strict';

//get user comments and posts
router
    .route("/api/user/getUserCommentsAndPosts")
    .get(
        function(req, res) {

            /*define variable to run sendResponse() function after completed both comment and post query*/
            var commentProcess = 0; //comment process not completed
            var postProcess = 0;   //post process not completed

            /*for store comments and post data in this variable for use in sendResponse() function*/
            var commentGlobal;
            var postGlobal;


            Comments.find({ user_id: req.payload.id }, function(err, CommentsData) {
                if (err) {
                    res.json({ status: 0, code: 200, type: "error", message: err });
                } else {

                    commentProcess = 1; //comment process is completed
                    commentGlobal = CommentsData; //assign local object to global object
                    sendResponse(); // call this function for send api response
                }
            });


            Posts.find({ user_id: req.payload.id }, function(err, PostsData) {
                if (err) {
                    res.json({ status: 0, code: 200, type: "error", message: err });
                } else {

                    postProcess = 1; //post process not completed
                    postGlobal = PostsData; //assign local object to global object
                    sendResponse(); // call this function for send api response
                }
            });

            //run this function after every process
            var sendResponse = function() {
                // check for all process is completed  if completed then send api response
                if (commentProcess !== 0 && postProcess !== 0) {
                    var data ={comments : commentGlobal, posts : postGlobal};
                    res.json({ status: 1, code: 200, type: "success", data: data });
                }
            };

        });

I do not want to make query in comment and post in step by step , for this reason I can not say which process will completed at last.

as describe above think, I have to make this type of code.

Can any body give me a guidelines to improve this code.

Thanks.

Answer 1

Here is my version of your code. It uses promises so we can make parallel request to get posts and comments.

const express = require("express");
const router = express.Router();
const Comments = require('../models/Comments');
const Posts = require('../models/Posts');

//use in strict mode
'use strict';

//get user comments and posts
router
  .route("/api/user/getUserCommentsAndPosts")
  .get(
    function (req, res) {
      const userId = req.payload.id; // use const is better than var

      // Do parallel requests using promise.all, it speed up your app indeed
      Promise.all([
        findCommentsByUserId(userId),
        findPostsByUserId(userId)
      ]).then((comments, posts) => {        
        res.json({
          status: 1,
          code: 200,
          type: "success",
          data: { comments, posts }
        });
      })
      .catch(err => {
        res.json({
          status: 0,
          code: 200,
          type: "error",
          message: err
        });
      });      
    });

/**
 * Find comments by user Id
 * - We make this function as promise so we later can do parallel request
 * - We move it to function to make your code in router cleaner and easier to read
 * @param userId 
 */
function findCommentsByUserId(userId) {
  return new Promise((resolve, reject) => {  
    Comments.find({
      user_id: userId
    }, function (err, CommentsData) {
      if (err) {
        reject(err);
      }        

      resolve(CommentsData);      
    });
  });
}

/**
 * Find posts by user Id
 * We make this function as promise so we can later can do parallel request
 * @param userId 
 */
function findPostsByUserId(userId) {
  return new Promise((resolve, reject) => {  
    Posts.find({
      user_id: userId
    }, function (err, PostsData) {
      if (err) {
        reject(err);
      }        

      resolve(PostsData);      
    });
  });
}

Hope it helps

Answer 2

If you have few (2 or 3) async operations then you can use promise chaining os that when first call succees another starts.

If you have more than two async operations OR as a better practice, you can use async library. If you all async operations are independent then user async.parallel. If you want them to execute in specific order then user async.waterfall

please check : https://github.com/caolan/async

Answer 3

Here is some refactoring with async waterfall when result from previous execution is passed to the next

var express = require("express");
var router = express.Router();
var Comments = require('../models/Comments');
var async = require('async');
var Posts = require('../models/Posts');

//use in strict mode
'use strict';

//get user comments and posts
router
    .route("/api/user/getUserCommentsAndPosts")
    .get(function(req, res) {

    async.waterfall([
        function(callback) {

            Comments.find({ user_id: req.payload.id }, function(err, CommentsData) {
                if (err) {
                    callback(err);
                } else {

                    var commentProcess = 1; //comment process is completed
                    var commentGlobal = CommentsData; //assign local object to global object

                    callback(null, commentProcess, commentGlobal);
                }
            });


        },
        function(commentProcess, commentGlobal, callback) {
            Posts.find({ user_id: req.payload.id }, function(err, PostsData) {
                if (err) {
                    callback(err);
                } else {

                    var postProcess = 1; //post process not completed
                    var postGlobal = PostsData; //assign local object to global object

                    callback(null, commentProcess, commentGlobal, postProcess, postGlobal);

                }
            });
        }
    ], function (err, commentProcess, commentGlobal, postProcess, postGlobal) {
        if (err) {
            res.json({ status: 0, code: 200, type: "error", message: err });
        } else {
            var data ={comments : commentGlobal, posts : postGlobal};
            res.json({ status: 1, code: 200, type: "success", data: data });
        }
    });


});

Answer 4

You need some sort of async control library. As stated by @binariedMe you can use bluebird which is a promise based library, or you could use async which is a set of different control flow methods with centralized error handling.