views:

730

answers:

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);
}
+3  A: 

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.

meouw
+3  A: 

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
I should have said why
meouw
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
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
A: 

Double your slashes.

new RegExp('\\$'+placeholdername+'\\$');
David Morton
+1  A: 

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