Code Optimization

[SumitKumar]

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

[Test Screen Name]

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.

[SumitKumar]

Thank you @Test Screen Name for your suggestion, I will keep in mind.

[Mike Bro]

Hello @SumitKumar,

You could write you code using a Switch Statement that contains multiple conditions.

Regards,

Mike

[SumitKumar]

Thank you @Mike Bro.

[Peter Kahrel]

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.

[SumitKumar]

Yes, you are right. I will keep in mind.

Thank you very much.

[Peter Kahrel · Accepted solution]

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';

[SumitKumar]

Thank you Peter for your suggestion.

[brian_p_dts]

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. 

[Dirk Becker]

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 …

[Peter Kahrel]

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.

[Dirk Becker]

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.

[Peter Kahrel]

Let Sumit Chaurasia try it. He or she asked the question!

P.

[SumitKumar]

Yes, I tried outside regex literal to test() method and got performance as you told.

Sumit

[Marc Autret]

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

[SumitKumar]

Thank you Marc, 

Your code always encourage to visit your website and learn new technique to solve the problem.

I also learned lots from here.

Regards,

Sumit