Copy link to clipboard
Copied
Hi All,
Is there way to write below code in short format?
All the conditions have 4 sub conditions.
if (jName.match(/mnap|pnqr|sopd/i)) {
if (aType.match(/article/i))
tName = "-Article_Blue.indt";
else if (aType.match(/editorial/i))
tName = "-Editorial_Blue.indt";
else if (aType.match(/review/i))
tName = "-Review_Blue.indt";
else
tName = "-Article_Blue.indt";
}
else if (jName.match(/jmng|nlpm|mnop/i)) {
if (aType.match(/article/i))
tName = "-Article_Green.indt";
else if (aType.match(/editorial/i))
tName = "-Editorial_Green.indt";
else if (aType.match(/review/i))
tName = "-Review_Green.indt";
else
tName = "-Article_Green.indt";
}
else if (jName.match(/jlps|zpmn|cvlo/i)) {
if (aType.match(/article/i))
tName = "-Article_Yellow.indt";
else if (aType.match(/editorial/i))
tName = "-Editorial_Yellow.indt";
else if (aType.match(/review/i))
tName = "-Review_Yellow.indt";
else
tName = "-Article_Yellow.indt";
}
and many more conditions with 4 sub conditions ...
--Sumit
Well, in fact this is shorter:
tName = '';
if (aType.test(/article/i))
tName = "-Article";
else if (aType.test(/editorial/i))
tName = "-Editorial";
else if (aType.test(/review/i))
tName = "-Review";
else
tName = "-Article";
if (jName.test(/mnap|pnqr|sopd/i)) {
tName += '_Blue.indt';
else if (jName.test(/jmng|nlpm|mnop/i)) {
tName += '_Green.indt';
else if (jName.test(/jlps|zpmn|cvlo/i)) {
tName += '_Yellow.indt';
Copy link to clipboard
Copied
Be careful of the word "optimization". This usually means to make the code faster. It does not mean to make the code smaller or easier to type. You could change this code to be table driven, and it would be smaller, but it would be slower.
Copy link to clipboard
Copied
Thank you @Test Screen Name for your suggestion, I will keep in mind.
Copy link to clipboard
Copied
Hello @SumitKumar,
You could write you code using a Switch Statement that contains multiple conditions.
Regards,
Mike
Copy link to clipboard
Copied
Thank you @Mike Bro.
Copy link to clipboard
Copied
I don't think you could do it much shorter and maintain readability. But you could make it a lot quicker by replacing .match() with .test(). Each match() call creates an array, which is a lot of overhead. Call to test() return true or false, which is way more efficient.
Copy link to clipboard
Copied
Yes, you are right. I will keep in mind.
Thank you very much.
Copy link to clipboard
Copied
Well, in fact this is shorter:
tName = '';
if (aType.test(/article/i))
tName = "-Article";
else if (aType.test(/editorial/i))
tName = "-Editorial";
else if (aType.test(/review/i))
tName = "-Review";
else
tName = "-Article";
if (jName.test(/mnap|pnqr|sopd/i)) {
tName += '_Blue.indt';
else if (jName.test(/jmng|nlpm|mnop/i)) {
tName += '_Green.indt';
else if (jName.test(/jlps|zpmn|cvlo/i)) {
tName += '_Yellow.indt';
Copy link to clipboard
Copied
Thank you Peter for your suggestion.
Copy link to clipboard
Copied
I'd do something like this:
var getColorByType = function (jName) {
jName = new RegExp(jName);
if (jName.test(
/mnap|pnqr|sopd/i
)) { return "Blue; }
//etc
}
var getTemplate = function (jName, aType) {
var color = getColorByType(jName);
aType = new RegExp(aType);
if (aType.test(/article/i)) { return "-Article_" + color + ".indt");
if (aType.test(/editorial/i) { return "-Editorial_" + color + ".indt");
//etc
}
var template = getTemplate ("mnap," "article");
Pardon formatting; on mobile.
Copy link to clipboard
Copied
If you're optimizing execution time e.g. inside a loop, it may help to compile the RegExp literals outside and reuse.
I would also try to use toLowerCase() on the left side variables once, rather than have the ignore-case in the RegExp.
$.hiresTimer will tell the difference …
Copy link to clipboard
Copied
Yes, timing is always a good idea. The below script shows that using toLowerCase() and indexOf is the slowest. Compiling a regex at every iteration cuts execution time by half. And precompiling a regex literal cuts the time down dramatically (but then, we knew that precompiling regexes is always a good idea).
s = 'xyz Editorial abc';
re = /editorial/i;
$.hiresTimer;
for (i = 0; i < 100; i++) {
s.toLowerCase().indexOf(s);
}
$.writeln ($.hiresTimer/1000000); // 0.002049
for (i = 0; i < 100; i++) {
/editorial/i.test(s);
}
$.writeln ($.hiresTimer/1000000); // 0.001200
for (i = 0; i < 100; i++) {
re.test(s);
}
$.writeln ($.hiresTimer/1000000); // 0.000451
P.
Copy link to clipboard
Copied
Peter, could you please also measure:
si = s.toLowerCase();
re = /editorial/; // no case-insensitive comparison
re = /^editorial$/; // no iterating thru the string
for (i = 0; i < 100; i++) {
re.test(si);
}
Edit: of course the second re only makes sense if the tested string is an exact match.
Copy link to clipboard
Copied
Let Sumit Chaurasia try it. He or she asked the question!
P.
Copy link to clipboard
Copied
Yes, I tried outside regex literal to test() method and got performance as you told.
Sumit
Copy link to clipboard
Copied
My two pennies:
// Incoming strings: `aType`, `jName` ; e.g:
aType = "xxxREVIEWyyy";
jName = "xyz-nlpm-abc";
const o={Blue:/mnap|pnqr|sopd/,Green:/jmng|nlpm|mnop/,Yellow:/jlps|zpmn|cvlo/};
var s, k;
// Suffix (color.)
s = jName.toLowerCase();
for( k in o ) if( o[k].test(s) ) break;
tName = '_' + k + '.indt';
// Prefix key.
0 <= (s=aType.toLowerCase()).indexOf(k='article') || 0 <= s.indexOf(k='editorial')
|| 0 <= s.indexOf(k='review') || (k='article');
// Result.
tName = '-' + k.charAt(0).toUpperCase() + k.slice(1) + tName;
alert( tName ); // => "-Review_Green.indt"
Best,
Marc
Copy link to clipboard
Copied