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",
}
}
]
}
1 Answer 1
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.
-
\$\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\$Jenny M– Jenny M2016年08月22日 15:05:26 +00:00Commented Aug 22, 2016 at 15:05
-
\$\begingroup\$ Maybe you have an object and not an array, let me update the code. \$\endgroup\$Mario Santini– Mario Santini2016年08月22日 15:58:24 +00:00Commented 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\$Jenny M– Jenny M2016年08月22日 17:12:16 +00:00Commented Aug 22, 2016 at 17:12
-
\$\begingroup\$ Object.values is available since Chrome 51 and Firefox 47 \$\endgroup\$woxxom– woxxom2016年08月22日 20:34:26 +00:00Commented Aug 22, 2016 at 20:34
provider
isn't used, so the loop looks meaningless. \$\endgroup\$