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.

I wrote a extender function to observableArray in knockout js. What I'm looking for is a way to extract the intersection records based on one property. This works well on smaller number of arrays, but shows pretty poor performance in larger arrays. I dont know if it is the observables that makes it slower?

How would you guys write it?

obsArray2 - The array to compare with

Key - Name of the property to compare between

ko.observableArray.fn.intersect = function (obsArray2, Key) {
    var self = Enumerable.From(ko.unwrap(this));
    var arrValues1 = self.Select(function (item) { return ko.unwrap(item[Key]); });
    arrValues2 = Enumerable.From(ko.unwrap(obsArray2)).Select(function (item) { return ko.unwrap(item[Key]); });
    var IntersectedValues = arrValues1.Intersect(arrValues2).ToArray();

    return self.Where(function (item) { return IntersectedValues.contains(ko.unwrap(item[Key])) }).ToArray();
    return obsArray1.Intersect(obsArray2).ToArray();
};
share|improve this question

1 Answer 1

I would not use linq.js for anything time sensitive, it is known to be slow, definitely compared to just using old skool for loops and arrays. Which is what I would use.

Other than, if I were to write this, I would consider naming some thing differently:

  • obsArray2 -> observableArray, there is no obsArray1, and it was not clear at first if obs means there are objects in the array or it is an observable arrays.
  • self -> anything but self which usually refers to this, perhaps enum1 ?
  • arrValues1 really contains an Enumerable which is not really an array, confusing, I would perhaps have chosen set1 because in maths, intersections are made of sets or enum1 because that is what the variable type is
  • Same for arrValues2 of course
  • intersectedValues -> I would have gone for simply intersection

Furthermore,

  • You have 2 return statements, there can be only one
  • It seems silly to go back to self and filter out only elements that are in the intersection, can't you build the return value from intersectedValues ?
share|improve this answer
    
Thank you. Naming variables is a skill i really lack. Thank you for great pointers! What would you use linqjs for? Smaller tasks? –  Pochen Dec 5 '14 at 8:40
    
I would use linqjs for complex operations with little volume. –  konijn Dec 5 '14 at 15:34

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.