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 am sorting an array (filtered) containing JS-Objects by their attribute sortdist:

filtered.sort(function (a, b) {
  if (!a.sortdist) { return 1;}
  else if (!b.sortdist) { return -1;}
  else if (a.sortdist > b.sortdist) { return 1;}
  else { return -1; }
});

However, I am sorting 9000 datasets, which takes a really long time. Is there any way to improve the performance of my sorting algorithm?

share|improve this question
    
What data type is sortdist? Can they be equal? What if they are both undefined? –  RoToRa Mar 7 at 11:15
    
Is this sort being done on the database, server side, or client side? –  Pete Mar 12 at 1:01
    
This post might be useful: stackoverflow.com/questions/8082425/… This guy implemented radix sort that can sort 2 million records. As well, you may want to look into using Typed Arrays (developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/…). –  Pete Mar 12 at 1:11

2 Answers 2

Disclaimer: I don't know anything about javascript

If you have a construct like

if (condition){
   return someValue;
} else { 
   return someOtherValue;
}

the else can be omited, because if the first condition is true the else is never reached.

So your code could be refactored to

filtered.sort(function (a, b) {
  if (!b.sortdist) { return -1; }
  if (!a.sortdist || a.sortdist > b.sortdist) { return 1; }
  return -1;
});  

which doesn't improve the performance, but I don't see anything to make your code faster.

share|improve this answer

Your results can be one of two. You can simplify the code by trying to meet one condition and return the expected result, otherwise, throw up the other result. In this case, your code only returns 1 when:

  • a.sortdist doesn't exist.
  • a.sortdist > b.sortdist.

...and -1 for all others. Thus your code can be simplified as:

filtered.sort(function (a, b) {
  return (!a.sortdist || a.sortdist > b.sortdist) ? 1 : -1;
});

As for performance, I suppose you could have negligible performance gain because you're not accessing props that much, and you're making the JS run less conditions. But for readability, this one's much more clearer.

share|improve this answer
    
Looks much better! But tested (jsperf.com/testswqeqwe) it doesnt really improve the performance. –  user3351652 Mar 6 at 10:18
    
@user3351652 Yup, performance gain is negligible. That's because we only removed statements in the sorter. The real bottleneck is the internal algorithm used by sort. You can search for custom sort implementations though. –  Joseph the Dreamer Mar 6 at 10:46

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.