4

In the code below. I expected true but i am getting false instead. What am I missing?

var text = "Sentence $confirmationlink$ fooo";     
alert(placeHolderExists(text,'confirmationlink'); // alerts false
function placeHolderExists(text,placeholdername) {  
  var pattern = new    RegExp('\$'+placeholdername+'\$');    
  return pattern.test(text);
}
Sergio del Amo
  • 76,835
  • 68
  • 152
  • 179

4 Answers4

14

The "\" in the RegExp expression builder is treated as an escape character when building the string, just as it is in the actual RegExp. You need to escape twice, try:

new RegExp('\\$'+placeholdername+'\\$');
annakata
  • 74,572
  • 17
  • 113
  • 180
  • Why use Regex if all Sergio needs is to verify the match exists? If Sergio wants the matched value then Regex should be used, but to just verify a match then IndexOf would be more effecient and less complex... No? – Dscoduc Feb 05 '09 at 20:34
  • 1
    Don't ask me, I only work here man. (Seriously, indexOf is approx 2x as fast a regex, but it's inflexible, and w're talking about something which is already *extremely* fast) – annakata Feb 05 '09 at 20:59
4

Should be

function placeHolderExists(text,placeholdername) {  
  var pattern = new    RegExp('\\$'+placeholdername+'\\$');    
  return pattern.test(text);
}

You need to double escape your $ signs

EDIT:
annakata explains why.

Community
  • 1
  • 1
meouw
  • 41,754
  • 10
  • 52
  • 69
4

This confusion is another example of why you shouldn't use regex unless you really need to.

return text.indexOf('$'+placeholdername+'$')!=-1;

...is simpler, quicker, and doesn't fall over when you have funny characters in it.

bobince
  • 528,062
  • 107
  • 651
  • 834
  • In this case (the original question) I would agree. Since you are really only looking for a true/false then instanceOf would be sufficient. However, if you wanted the value of the match then regex would be the correct tool for the job. – Dscoduc Feb 05 '09 at 20:33
1

Double your slashes.

new RegExp('\\$'+placeholdername+'\\$');
David Morton
  • 16,338
  • 3
  • 63
  • 73