Should i use promise or callback in the following code?

user_11646579 0
Viewed 92

I have some routes

routes.js

var express = require("express");
var router = express.Router();

const { controllerMethod1, controllerMethod2 } = require("./controller");

router.get("/route1", controllerMethod1);
router.get("/route2", controllerMethod2);

module.exports = router;

if i use promise variable as global, its used by all method in controller.js. should i use global or local variable for promise ?

controller.js

const {
  serviceMethod1,
  serviceMethod2,
  serviceMethod1ByDate,
} = require("./services");

let promise; //global promise variable
const controllerMethod1 = (req, res) => {
  //let promise; local promise variable

  //This is for Callback
  if (req.query.date) {
    serviceMethod1ByDate(req.query.date, (err, result) => {
      if (err) {
        res.status(500).json({
          status: "error",
          message: "error using callback",
        });
      }
      if (result) {
        res.status(200).json({
          status: "success",
          message: "success using callback",
        });
      }
    });
  } else {
    serviceMethod1((err, result) => {
      if (err) {
        res.status(500).json({
          status: "error",
          message: "error using callback",
        });
      }
      if (result) {
        res.status(200).json({
          status: "success",
          message: "success using callback",
        });
      }
    });
  }

  // This is for Promise
  promise = req.query.date
    ? serviceMethod1ByDate(req.query.date)
    : serviceMethod1();

Should i use way 1 or way 2 ? if multiple users request one or more routes at the same time,can handleResponse method work correctly?

Way 1 for promise

promise
    .then((results) => {
      return res.json({
        status: "success with promise variable",
        data: results,
      });
    })
    .catch((error) => {
      return res.status(500).json({
        status: "error with promise variable",
        message: "there is no person details",
      });
    });

Way 2 for Promise

  handleResponse(promise, res);

//this method is working for all routes when i use promise
const handleResponse = (results, response) => {
  results
    .then((result) => {
      return response.json({
        status: "success with promise variable in handleResponse",
        data: result,
      });
    })
    .catch((error) => {
      return response.status(500).json({
        status: "error with promise variable handleResponse",
        message: "Internal Server Error",
      });
    });
};

controller.js

const controllerMethod2 = (req, res) => {
  //------------------ Using Callback Method -------------
  serviceMethod2((err, result) => {
    if (err) {
      res.status(500).json({
        status: "error",
        message: "error using callback",
      });
    }
    if (result) {
      res.status(200).json({
        status: "success",
        message: "success using callback",
      });
    }
  });

  //------------------ Using Promise Method -------------
  //local variable
  let promise;
  promise = serviceMethod2();

  //Way 1 for Promise
  promise
    .then((result) => {
      //...
    })
    .catch((err) => {
      //...
    });

  //Way 2 for Promise
  handleResponse(promise, res);
};

module.exports = { controllerMethod1, controllerMethod2 };

service.js

const pool = require("../../../config/database");

//-----------------------Using Callback Mehthod----------------

const serviceMethod1 = async (CallBack) => {
  let query = "select * from databse";
  await pool.query(query, [], (error, results, fields) => {
    if (error) {
      return CallBack(error);
    }
    return CallBack(null, results);
  });
};

const serviceMethod1ByDate = async (date) => {
  let query = "select * from databse where date ?";
  return await new Promise((resolve, reject) => {
    pool.query(query, [date], (error, results, fields) => {
      if (error) {
        return CallBack(error);
      }
      return CallBack(null, results);
    });
  });
};

const serviceMethod2 = async (Callback) => {
  let query = "select * from database";
  await pool.query(query, [], (error, results, fields) => {
    if (error) {
      return CallBack(error);
    }
    return CallBack(null, results);
  });
};

//-----------------------Using Promise Method----------------

const serviceMethod1 = async () => {
  let query = "select * from databse";
  return await new Promise((resolve, reject) => {
    pool.query(query, [], (error, results, fields) => {
      if (results) {
        resolve(results);
      } else {
        reject(error);
      }
    });
  });
};

const serviceMethod1ByDate = async (date) => {
  let query = "select * from databse where date ?";
  return await new Promise((resolve, reject) => {
    pool.query(query, [date], (error, results, fields) => {
      if (results) {
        resolve(results);
      } else {
        reject(error);
      }
    });
  });
};

const serviceMethod2 = async () => {
  let query = "select * from database";
  return await new Promise((resolve, reject) => {
    pool.query(query, [], (error, results, fields) => {
      if (results) {
        resolve(results);
      } else {
        reject(error);
      }
    });
  });
};

module.exports = {
  serviceMethod1,
  serviceMethod1ByDate,
  serviceMethod2,
};
promise node.js callback javascript performance
1 Answers

if i use promise variable as global, its used by all method in controller.js. should i use global or local variable for promise ?

You should use local variable for this type of operation as global variable are generally used to define constants or methods. They cannot be used as temporary values because it's value can be changed anytime and that'll result in conflict with other functionalities and so must be avoided.

Should i use way 1 or way 2 ? if multiple users request one or more routes at the same time,can handleResponse method work correctly?

Way 2 is more efficient that Way 1 because if you use way 1 then you will have to do it for every method in the controller. Way 2 is like a common method where you can format your response and most of the developers use it.

It doesn't make any difference whether you use callbacks or promises but just a clean way to do things.

Instead of using this:

const serviceMethod1 = async () => {
  let query = "select * from databse";
  return await new Promise((resolve, reject) => {
    pool.query(query, [], (error, results, fields) => {
      if (results) {
        resolve(results);
      } else {
        reject(error);
      }
    });
  });
};

Use this:

// Remove the async await from here and handle the response/error where the below method is called by putting it in try catch block.

const serviceMethod1 = () => {
  let query = "select * from databse";
  return new Promise((resolve, reject) => {
    pool.query(query, [], (error, results, fields) => {
      if (results) {
        resolve(results);
      } else {
        reject(error);
      }
    });
  });
};


// otherFile.js

someMethod = async () => {
  try {
    const result = await serviceMethod1();
    // handle the response
  } catch {
    // handle the error
  }
}
user_83852080
Related