26

I have code similar to this filtering entries in an Array of Objects:

var filterRegex = new RegExp(".*blah.*","ig");
if (filterRegex.test(events[i].thing) && events[i].show) {
    console.log("SUCCESS: filtering thing " + i + " " + events[i].thing);
    events[i].show = false;
    numevents--;
}

I get inconsistent results with this if condition (checking with Firebug, both conditions are true individually, but sometimes the whole expression evaluates to false). HOWEVER, if I actually put an alert() called inside this if statement (like line 4), it becomes consistent and I get the result I want.

Can you see anything wrong with this logic and tell me why it's not always producing what is expected?

Shog9
  • 156,901
  • 35
  • 231
  • 235
Eric Wendelin
  • 43,147
  • 9
  • 68
  • 92
  • I think you need to elaborate a bit and maybe put the code in a bit more context. For instance; why do you reference both events[i] and events[0] in this code? It seems inconsistent. – erlando Oct 16 '08 at 18:42
  • @erlando... sorry, I was playing around with the code in Firebug, but I assure you I'm referencing i instead of 0 in all cases. – Eric Wendelin Oct 16 '08 at 19:11

3 Answers3

62

Ok, i see it now. The key to your problem is the use of the g (global match) flag: when this is specified for a regex, it will be set up such that it can be executed multiple times, beginning each time at the place where it left off last time. It keeps a "bookmark" of sorts in its lastIndex property:

var testRegex = /blah/ig;
// logs: true 4
console.log(testRegex.test("blah blah"), testRegex.lastIndex);
// logs: true 9 
console.log(testRegex.test("blah blah"), testRegex.lastIndex);
// logs: false 0
console.log(testRegex.test("blah blah"), testRegex.lastIndex);

The above example creates an instance of a very simple regex: it matches "blah", upper or lower case, anywhere in the string, and it can be matched multiple times (the g flag). On the first run, it matches the first "blah", and leaves lastIndex set to 4 (the index of the space after the first "blah"). The second run starts matching at the lastIndex, matches the second blah, and leaves lastIndex set to 9 - one past the end of the array. The third run doesn't match - lastIndex is bogus - and leaves lastIndex set to 0. A fourth run would therefore have the same results as the first.

Now, your expression is quite a bit more greedy than mine: it will match any number of any characters before or after "blah". Therefore, no matter what string you test on, if it contains "blah" it will always match the entire string and leave lastIndex set to the length of the string just tested. Meaning, if you were to call test() twice, the second test would always fail:

var filterRegex = /.*blah.*/ig;
// logs: true, 9
console.log(filterRegex.test("blah blah"), filterRegex.lastIndex);
// logs: false, 0 
console.log(filterRegex.test("blah blah"), filterRegex.lastIndex);

Fortunately, since you create your regex immediately prior to calling test(), and never call test() more than once, you'll never run into unexpected behavior... Unless you're using a debugger that lets you add in another call to test() on the side. Yup. With Firebug running, a watch expression containing your call to test() will result in intermittent false results showing up, either in your code or in the watch results, depending on which one gets to it first. Driving you slowly insane...

Of course, without the g flag, livin' is easy:

var filterRegex = /.*blah.*/i;
// logs: true, 0
console.log(filterRegex.test("blah blah"), filterRegex.lastIndex);
// logs: true, 0 
console.log(filterRegex.test("blah blah"), filterRegex.lastIndex);

Suggestions

  • Avoid the global flag when you don't need it.
  • Be careful what you evaluate in the debugger: if there are side effects, it can affect the behavior of your program.
Shog9
  • 156,901
  • 35
  • 231
  • 235
  • 1
    Holy crap! KUUUUDOs man, you're absolutely correct! Thank you so much! – Eric Wendelin Oct 16 '08 at 20:21
  • 1
    I was as shocked as you when i copied your code into a test page, put in a Firebug breakpoint, and saw the watches coming back false. :-) I've never used g with test() before, and hadn't realized this is how it behaved! – Shog9 Oct 16 '08 at 20:25
  • 2
    Impressive. That made my jaw drop as well. :-) – Tomalak Oct 16 '08 at 20:45
  • Thanks for spelling that out! I got caught out by this while using the same regex to test multiple phone number fields. Couldn't figure out why every 2nd field would fail. Removing the global flag fixed it. – Scott McClung Oct 23 '17 at 18:53
0

I just can't imagine there is any situation where two JavaScript expressions evaluate to true individually, but not when combined.

Are you sure both expressions actually produce a boolean value every time? (Okay, to make regex.test() not produce a boolean value is difficult, but how about event.show. Might that be undefined at times?

Do you refer to the correct index when saying event[0].show, wouldn't you mean event[i].show?

Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • Oops, I do know I am referencing "i" instead of 0. Sorry I copied a bit of test code from Firebug that is present from my function. event.show is always boolean, but I'll double-check that it's always defined and let you know. Thanks. – Eric Wendelin Oct 16 '08 at 19:08
0

That's seems you are facing some kind of race conditions with the event array, that's why when you use the alert() everything works fine.

Gravstar
  • 1,071
  • 6
  • 10
  • I think so too, but the really weird thing is that it is idempotent, so it's inconsistent, consistently. Given the same data it fails the exact same way every time. Could that be caused by a race condition? Isn't javascript single-threaded anyway? – Eric Wendelin Oct 16 '08 at 19:10
  • I can't imagine a race condition either. I suppose you are just running a loop over an array of objects, why would something that simple fail. – Tomalak Oct 16 '08 at 19:15
  • That, my friend, is the $10 question... – Eric Wendelin Oct 16 '08 at 19:47
  • Well, set up a minimal test case then, make that fail consistently in your setup and post it here for people to see. The snippet you posted really cannot give a clear indication where things go wrong. – Tomalak Oct 16 '08 at 20:13