Welcome Dialog

Welcome to the Community!

We have a brand new look! Take a tour with us and explore the latest updates on Adobe Support Community.


Code Optimization

Engaged ,
Jul 18, 2021 Jul 18, 2021

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

-Sumit
TOPICS
How to, Performance, Scripting, SDK, Server developers

Views

214

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines

correct answers 1 Correct answer

Adobe Community Professional , Jul 18, 2021 Jul 18, 2021
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';

Likes

Translate

Translate
LEGEND ,
Jul 18, 2021 Jul 18, 2021

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.

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 18, 2021 Jul 18, 2021

Copy link to clipboard

Copied

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

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Advocate ,
Jul 18, 2021 Jul 18, 2021

Copy link to clipboard

Copied

Hello @SumitKumar,

 

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

 

Regards,

Mike

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 18, 2021 Jul 18, 2021

Copy link to clipboard

Copied

Thank you @mikeb41294032.

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Adobe Community Professional ,
Jul 18, 2021 Jul 18, 2021

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.

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 18, 2021 Jul 18, 2021

Copy link to clipboard

Copied

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

 

Thank you very much.

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Adobe Community Professional ,
Jul 18, 2021 Jul 18, 2021

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

 

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 18, 2021 Jul 18, 2021

Copy link to clipboard

Copied

Thank you Peter for your suggestion.

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Adobe Community Professional ,
Jul 18, 2021 Jul 18, 2021

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. 

 

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Jul 18, 2021 Jul 18, 2021

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 …

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Adobe Community Professional ,
Jul 19, 2021 Jul 19, 2021

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.

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Jul 19, 2021 Jul 19, 2021

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.

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Adobe Community Professional ,
Jul 19, 2021 Jul 19, 2021

Copy link to clipboard

Copied

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

 

P.

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 19, 2021 Jul 19, 2021

Copy link to clipboard

Copied

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

 

Sumit

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Guide ,
Jul 22, 2021 Jul 22, 2021

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

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Engaged ,
Jul 25, 2021 Jul 25, 2021

Copy link to clipboard

Copied

LATEST

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

-Sumit

Likes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines