CODEWITHSUNDEEP
javascriptstringparsingrecursion
tay10r
tay10r

Reputation: 4357

program with recursive function

I'm pretty stumped on this recursive function. I'm using it to highlight words in a text box, but it's giving me some strange output:

 should be:
 #define
 testing #define

 but instead it's:

 testing #define
 testing #define

here's the code

function replace_all( text, old_str, new_str ){

    index_of = text.indexOf( old_str );

    if( index_of != -1 ){

            old_text = text.substring( 0, index_of );
            new_text = text.substring( index_of + old_str.length );

            new_text = replace_all( new_text, old_str, new_str );

            text = old_text + new_str + new_text;
    }
    return text;
}

Any ideas on what's wrong with the function? It seems to be replacing all the old keywords with the last one found.

Upvotes: 0

Views: 83

Answers (2)

Niet the Dark Absol
Niet the Dark Absol

Reputation: 324650

Converting my comment to an answer:

This would be a lot easier like so:

function replace_all(text,old_str,new_str) {
    var old_regex = old_str).replace(/[.\\+*?\[\^\]$(){}=!<>|:-]/g, '\\$&');
    // the above line escapes any characters that may interfere with regex behaviour
    return text.replace(new RegExp(old_regex,"g"),new_str);
}

Upvotes: 1

jfriend00
jfriend00

Reputation: 707436

You need to at least declare all your variables in the function as local variables by adding var in front of their first use. 

function replace_all( text, old_str, new_str ){

    var index_of = text.indexOf( old_str );

    if( index_of != -1 ){

            var old_text = text.substring( 0, index_of );
            var new_text = text.substring( index_of + old_str.length );

            new_text = replace_all( new_text, old_str, new_str );

            text = old_text + new_str + new_text;
    }
    return text;
}

By not using var, your variables are global and each invocation of replace_all will be sharing the same copies of variables which will mess up the recursion as recursive calls will mess up the state of the higher levels calls. If the variables are all local variables, then each function invocation has it's own set of variables and one recursive call does not mess up the others.

Plus, it is generally always a good idea to limit the scope of your variables to as local a scope as practical and avoid globals whenever possible. Implicit globals variables like you have here are particular evil because they can easily lead to accidental misuse.

As Kolink suggested, you might just want to do a single global .replace() operation using a regex search/replace. You would need to make sure that any regex special characters in the search string were properly escaped though.

Upvotes: 3

Related Questions