Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Using jQuery I'm trying to find an efficient way to iterate through an array excludeMe and check if elements from another array needsExclusions are contained in the excludeMe array. Not sure if this is the most efficient, but it works. Fiddle here: http://jsfiddle.net/yef9q/3/

//Iterate through needsExclusions for an excludeMe string
//someFlag raised on matching strings

var excludeMe = ["in west", "philadelphia", "born and", "raised"];
var needsExclusions = ["philadelphia", "raised"];
var someFlag = true;

$.each(excludeMe, function(i,val) {
  //alert("value: " + val);
  if ($.inArray(val, needsExclusions) != -1) {
      alert("matching value: " + val);
      someFlag = false;        
  }
});
share|improve this question
    
Sounds like your looking for an intersection algorithm –  megawac Jul 22 at 20:23
    
@megawac While the result can be determined from the intersection (it's not empty), the code above merely tests whether the two sets are disjoint which is faster to calculate when they have at least one element in common. –  David Harkness Jul 22 at 21:12
1  
@DavidHarkness it seems so, its hard to tell with that alert what the asker is looking for –  megawac Jul 22 at 21:24

2 Answers 2

up vote 3 down vote accepted

Your particular example gets 10 times faster by just removing the jQuery and using a for loop and .indexOf() instead of the jQuery versions. It's also worth stopping the iteration when you find an excluded value since you don't need to look any more once the flag is already set:

var excludeMe = ["in west", "philadelphia", "born and", "raised"];
var needsExclusions = ["philadelphia", "raised"];
var someFlag = true;

for (var i = 0; i < excludeMe.length; i++) {
    if (needsExclusions.indexOf(excludeMe[i]) !== -1) {
        someFlag = false;        
        break;
    }
}

Here's a jsperf that illustrates the speed difference: http://jsperf.com/exclusion-test

If you had a much large data set, then it might pay off to put the needsExclusions values into an object to serve as a direct lookup index (faster than array.indexOf()), but that's probably only faster when the list gets a lot longer because there's setup overhead to build the initial index.

share|improve this answer
    
added an ugly implementation to ur jsperf to just to be annoying (jsperf.com/exclusion-test/2) :) –  megawac Jul 22 at 20:32
    
very helpful, thank you! and I like that jsperf site, I had never seen that before. –  MannfromReno Jul 22 at 20:55
    
@MannfromReno - yeah, pretty much any performance related question can really only be answered with testing and jsperf is often very useful for performance comparisons. –  jfriend00 Jul 22 at 21:14
    
@megawac There's a bug in the while-loop version: it stops on the first non-match instead of the first match, but it's still faster once fixed. –  David Harkness Jul 22 at 22:04

Very similar to jfriend00's answer, just using a different setup for the loop:

var excludeMe = ["in west", "philadelphia", "born and", "raised"];
var needsExclusions = ["philadelphia", "raised"];
var someFlag = true;

for( var i = 0, l = excludeMe.length ; i < l && someFlag ; i++ ) {
  someFlag = needsExclusions.indexOf(excludeMe[i]) === -1;
}

If something from excludeMe is found in needsExclusion, someFlag gets set to false, and the loop exits, since it'll only loop while i < l && someFlag.

And yes, definitely skip jQuery. You're just working with simple arrays here, no need to bring in jQuery for that.

On another note: Your naming is very confusing. The two arrays are named so similarly that I had to do several double-takes before I knew what was what.

Given that it's an example, I don't know what the actual names should be, but I'd recommend something like someArray for the input, and blacklist for the items that must not be in someArray. Clearer that way.

share|improve this answer
    
thank you, yes I know I just threw together some random names, they're named completely different and more intuitive in my solution. –  MannfromReno Jul 22 at 20:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.