2
\$\begingroup\$

The code's purpose is to call to some function async given some condition was true. I've using the following code which is working as expected. Is there a way to write it better, maybe by not using the promise push array?

var _run = (req, res, oModule, oConfig, urlPath,action) => {
 var aPromises = [];
 //requested action to call
 for (var provider in oConfig.providers[0]) {
 if ((oConfig.providers[0][provider].val === action))
 {
 var fnName = oConfig.providers[0][provider].function;
 //Run on provided actions
 if (typeof oModule[oConfig.providers[0][provider].function] === "function") {
 try {
 logger.info(`Function ${fnName} is called `);
 aPromises.push(oModule[fnName](req, res));
 }
 catch (err) {
 logger.error(`Error returned ${err}`);
 }
 }
 }
 }
 return Promise.all(aPromises);
};

update:

This is the value of config object and in the code I need to call to the function according to the path I got...

{
 "providers": [
 {
 "run": {
 "path": "command1",
 "function": "fn1",
 },
 "save": {
 "path": "test2",
 "function": "fn2",
 }
 }
 ]
}
asked Aug 22, 2016 at 7:10
\$\endgroup\$
2
  • \$\begingroup\$ The loop variable provider isn't used, so the loop looks meaningless. \$\endgroup\$ Commented Aug 22, 2016 at 8:04
  • \$\begingroup\$ @wOxxOm - done please have a look,( i just want to make it easy to read...) \$\endgroup\$ Commented Aug 22, 2016 at 8:48

1 Answer 1

2
\$\begingroup\$

Instead of using the for loop with if flow control, you could use the arrays functions to convert all more functional.

As your code looks broken I wrote my refactoring in the following test:

'use strict';
// This is just to run the test 
const request = {}
const response = {}
// This is a trivial oModule object
const M = {
 fn1: function() {
 return new Promise(() => console.log('fn1'))
 },
 fn2: function() {
 return new Promise(() => console.log('fn2'))
 }
}
// This is the oConfig object as you post in your OP
const C = {
 providers: [
 {
 "run": {
 "path": "command1",
 "function": "fn1",
 },
 "save": {
 "path": "test2",
 "function": "fn2",
 }
 }
 ]
}
// The proposed refactoring
const _run = (req, res, oModule, oConfig, urlPath, action) => {
 let aPromises = Object.keys(oConfig.providers[0])
 .filter(providerName => (providerName == action))
 .map(providerName => {
 let provider = oConfig.providers[0][providerName]
 return oModule[provider.function]
 })
 .filter(func => (typeof func === 'function'))
 .map(func => func(req, res))
 return Promise.all(aPromises)
}
// This run the test to 
_run(request, response, M, C, '', 'run')

I used 2 different filter/map run to check the 2 checks on the 2 different objects oConfig and oModule.

I tested this with nodejs v4.4.4 on windows and it works without errors.

It print out:

fn1

I removed the var and use const or let.

It should be the same, not sure if it is more readable.

You never use urlPath so you can remove from arguments.

I didn't put any ; as they are not mandatory in javascript, but if you prefere to use feel free to put at the end of each expression.

Also, I hope that oModule[provider.function](req, res) returns a Promise or your code is blocking, but maybe is so.

answered Aug 22, 2016 at 14:28
\$\endgroup\$
4
  • \$\begingroup\$ Thanks I've tried it and I got error oConfig.providers[0].filter is not a function ,but this is array... not sure that I got it... \$\endgroup\$ Commented Aug 22, 2016 at 15:05
  • \$\begingroup\$ Maybe you have an object and not an array, let me update the code. \$\endgroup\$ Commented Aug 22, 2016 at 15:58
  • \$\begingroup\$ Thank you , I still got error Object.values is not a function... I update the post with the full object maybe it can help, thanks for assistance... \$\endgroup\$ Commented Aug 22, 2016 at 17:12
  • \$\begingroup\$ Object.values is available since Chrome 51 and Firefox 47 \$\endgroup\$ Commented Aug 22, 2016 at 20:34

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.