JavaScript style/optimization: String.indexOf() v. Regex.test()

Question

I've recently come across this piece of JavaScript code:

if (",>=,<=,<>,".indexOf("," + sCompOp + ",") != -1)

I was intrigued, because to write this test I would have done:

if (/(>=|<=|<>)/.test(sCompOp))

Is this just a stylistic difference, or does the author of the other code know something about optimization that I don't? Or perhaps there is a different good reason to do this, or to not use regexes...?

It seems to me that using String.indexOf() for this is a little more difficult to read (but then, I'm quite comfortable with regular expressions), but are there instances where it might be "better" than writing an equivalent regex?

By "better" that might be quicker or more efficient, (although obviously that depends on the browser's JavaScript engine), or some other reason I'm not aware of. Can anyone enlighten me?

Answer 1

I ran some tests. The first method is slightly faster, but not by enough to make any real difference even under heavy use... except when sCompOp could potentially be a very long string. Because the first method searches a fixed-length string, its execution time is very stable no matter how long sCompOp gets, while the second method will potentially iterate through the entire length of sCompOp.

Also, the second method will potentially match invalid strings - "blah blah blah <= blah blah" satisfies the test...

Given that you're likely doing the work of parsing out the operator elsewhere, i doubt either edge case would be a problem. But even if this were not the case, a small modification to the expression would resolve both issues:

/^(>=|<=|<>)$/

---

Testing code:

function Time(fn, iter)
{
   var start = new Date();
   for (var i=0; i<iter; ++i)
      fn();
   var end = new Date();
   console.log(fn.toString().replace(/[\r|\n]/g, ' '), "\n : " + (end-start));
}

function IndexMethod(op)
{
   return (",>=,<=,<>,".indexOf("," + op + ",") != -1);
}

function RegexMethod(op)
{
   return /(>=|<=|<>)/.test(op);
}

function timeTests()
{
   var loopCount = 50000;
   
   Time(function(){IndexMethod(">=");}, loopCount);
   Time(function(){IndexMethod("<=");}, loopCount);
   Time(function(){IndexMethod("<>");}, loopCount);
   Time(function(){IndexMethod("!!");}, loopCount);
   Time(function(){IndexMethod("the quick brown foxes jumped over the lazy dogs");}, loopCount);
   Time(function(){IndexMethod("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<");}, loopCount);

   Time(function(){RegexMethod(">=");}, loopCount);
   Time(function(){RegexMethod("<=");}, loopCount);
   Time(function(){RegexMethod("<>");}, loopCount);
   Time(function(){RegexMethod("!!");}, loopCount);
   Time(function(){RegexMethod("the quick brown foxes jumped over the lazy dogs");}, loopCount);
   Time(function(){RegexMethod("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<");}, loopCount);
}

timeTests();

Tested in IE6, FF3, Chrome 0.2.149.30

Answer 2

I doubt it is a question of performance or optimization. I would suspect the author of that code was simply not comfortable or familiar with regular expressions. Also notice how the comma-separated string isn't split apart in order to leverage object properties - possibly also a case of lack of familiarity with the language.

For example, another way of testing for an operator in a comma-separated list of allowable operators would be to split the comma-separated list of allowable operators and do a one-time initialization of an object with the operators as properties:

var aOps = ">=,<=,<>".split(",");
var allowableOps = {};
for (var iLoop = 0; iLoop < aOps.length; iLoop++) {
  allowableOps[aOps[iLoop]] = true;
} //for

This small initialization overhead would likely be offset by the ability to do speedy lookups:

if (allowableOps[sCompOp]) { ... }

Of course, this could end up being slower overall, but it is arguably a cleaner approach.