3
\$\begingroup\$

I have a script that matches a string against ~20 different regexs for the purpose of changing it. They are ordered and formed that the string will only ever match against one. How can I avoid checking all the str.replaces after I match once?

My code here works, but I repeat myself a lot and it does not seem optimal.

The while loop exists so I could break, I don't actually need to loop through anything - and in fact I ensure I don't loop at the end of it. I'm merely looking for a way to stop performing replaces, when I know they will do nothing.

function format_number(input){
    var str = input.value;
    str.toString();
    var copy = str;
    var checked = false;
    while (!checked) {
        str = str.replace(/regex1/, 'replace1');
        if (copy != str) {break;}
        str = str.replace(/regex2/, 'replace2'); 
        if (copy != str) {break;}
        str = str.replace(/regex3/, 'replace3');
        if (copy != str) {break;}
        str = str.replace(/regex4/, 'replace4');
        if (copy != str) {break;}
        str = str.replace(/regex5/, 'replace5');
        if (copy != str) {break;}
        str = str.replace(/regex6/, 'replace6');
        if (copy != str) {break;}
        str = str.replace(/regex7/, 'replace7');
        if (copy != str) {break;}
        str = str.replace(/regex8/, 'replace8');
        if (copy != str) {break;}
        str = str.replace(/regex9/, 'replace9');
        if (copy != str) {break;}
        str = str.replace(/regex10/, 'replace10');
        if (copy != str) {break;}
        str = str.replace(/regex11/, 'replace11');
        if (copy != str) {break;}
        str = str.replace(/regex12/, 'replace12');
        if (copy != str) {break;}
        str = str.replace(/regex13/, 'replace13');
        if (copy != str) {break;}
        str = str.replace(/regex14/, 'replace14');
        if (copy != str) {break;}
        str = str.replace(/regex15/, 'replace15');
        if (copy != str) {break;}
        str = str.replace(/regex16/, 'replace16');
        if (copy != str) {break;}
        str = str.replace(/regex17/, 'replace17');
        if (copy != str) {break;}
        checked = true;
    }
    input.value=str;
}
\$\endgroup\$
7
  • \$\begingroup\$ When is checked set to true? Why is this an infinite loop at all? \$\endgroup\$
    – Bergi
    Jun 10, 2014 at 19:15
  • \$\begingroup\$ My mistake, I added the final line manually and put it in incorrectly. It should break after the first iteration. \$\endgroup\$
    – Devon Parsons
    Jun 10, 2014 at 19:42
  • \$\begingroup\$ Then what good is that loop at all? \$\endgroup\$
    – Bergi
    Jun 10, 2014 at 19:45
  • 2
    \$\begingroup\$ Hm, a labelled block would've done that as well… or using if-else-statements. Regardless, what is the goal of this code? To have checked be true or false? To modify str? \$\endgroup\$
    – Bergi
    Jun 10, 2014 at 19:49
  • 1
    \$\begingroup\$ I have a feeling that this code could be improved much more if only we could also see the actual regexes. It's hard for me to understand what kind secret proprietary replacements one could do inside a function called format_number. Or maybe the function doesn't do what it's name suggests. \$\endgroup\$
    – Rene Saarsoo
    Jun 11, 2014 at 17:09

1 Answer 1

Reset to default
11
\$\begingroup\$

This seems to be a fine case for a loop over the expressions:

function format_input(input) {
    // decouple DOM things from pure functionality!
    input.value = format_number(input.value);
}
function format_number(str) {
    var replacements = [
        [/regex1/, 'replace1'],
        [/regex2/, 'replace2'],
        [/regex3/, 'replace3'],
        [/regex4/, 'replace4'],
        [/regex5/, 'replace5'],
        [/regex6/, 'replace6'],
        [/regex7/, 'replace7'],
        [/regex8/, 'replace8'],
        [/regex9/, 'replace9'],
        [/regex10/, 'replace10'],
        [/regex11/, 'replace11'],
        [/regex12/, 'replace12'],
        [/regex13/, 'replace13'],
        [/regex14/, 'replace14'],
        [/regex15/, 'replace15'],
        [/regex16/, 'replace16'],
        [/regex17/, 'replace17']
    ];
    for (var i=0; i<replacements.length; i++) {
        var copy = str.replace(replacements[i][0], replacements[i][1]);
        if (str != copy)
            return copy;
    }
    return str;
}
\$\endgroup\$
0

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.