0

I have this function:

tests.sort(function (a, b) {
   var diff = a.title.localeCompare(b.title);
   return diff == 0 ? b.modifiedDate.localeCompare(a.modifiedDate) : diff;
});

I am using it to sort the tests array first by title and then by modifiedDate. The code was working but now I found it gives an error. When the modifiedDate is null and when this happens the compare fails.

How could I make it so that if the modifiedDate is null then the sort still works and places those rows after the rows with a modifiedDate that's not null?

Alan2
  • 23,493
  • 79
  • 256
  • 450
  • i sometimes trash the arguments as i work down the possibilities, so you can do something like `b=b.modifiedDate||0;a=a.modifiedDate||0;` before the compare. – dandavis Mar 18 '16 at 04:33
  • You might want to have a look at [this](http://stackoverflow.com/a/19103480/1048572) – Bergi Mar 18 '16 at 04:35
  • A simple `diff == 0 && b.modifiedDate ... ?` doesn't work? – vol7ron Mar 18 '16 at 04:36

2 Answers2

1

Thinking off the top of my head, to sort by title and then modifiedDate with nulls last:

tests.sort(function (a, b) {
   var diff = a.title.localeCompare(b.title),
       has_modifiedDate = a.modifiedDate && b.modifiedDate && true || false; 


  if (diff === 0) {
    if (has_modifiedDate) {
      return a.modifiedDate.localeCompare(b.modifiedDate)
    }
    else {
      if (! b.modifiedDate){
        if (! a.modifiedDate)
          return 0;
        else
          return -1;
      }
      else {
        if (! a.modifiedDate)
          return 1;
        else
          return -1;
      }
    }
  }
  else 
    return diff;
});

Note: this is untested and it's very late/early here. If this is incorrect, post and I'll delete or update; but way to tired to think.

Quick dataset you can try testing with; fill in with whatever data you want:

var tests = [
  {modifiedDate:'z',    title:'foo'},
  {modifiedDate:'a',    title:'foo'},
  {modifiedDate:null,   title:'foo'},
  {modifiedDate:'a',    title:'foo'},
  {modifiedDate:'null', title:'foo'},
  {modifiedDate:'z',    title:'foo'},
  {modifiedDate:'z',    title:'bar'},
  {modifiedDate:'a',    title:'bar'},
  {modifiedDate:null,   title:'bar'}
 ];
vol7ron
  • 40,809
  • 21
  • 119
  • 172
  • You could shorten it a lot (e.g. by omitting the second `if (! a.modifiedDate)`), but it appears to be correct – Bergi Mar 18 '16 at 14:17
  • I haven't tested, but I think it's needed to push nulls to the bottom of the stack, because it's saying if they both have the same title and the next element has a modifieddate, but the current element doesn't. It looks like the `if` could be standalone and the `else` wrapper isn't needed – vol7ron Mar 18 '16 at 15:42
  • Just curious: why the extra true before or? a.modifiedDate && b.modifiedDate && true || false. I haven't seen this approach before. – Will Mar 18 '16 at 16:49
  • @will it's not necessary. Without it, the variable will hold a "truthy" value, with it it will hold `true`. So, if you want to be more explicit in the if statement you could use the exact comparison operator and do `has_modifiedDate === true`. Probably has a very very micro performance benefit too, since the variable will hold a smaller value compared to a date. But, it's not necessary; it's a convention I started adopting, when I expect my `is_*` and `has_*` variables to only have `true` or `false` values. – vol7ron Mar 18 '16 at 17:47
  • @vol7ron I meant `else { return 1; }` would suffice as you already know that not both an not b are null – Bergi Mar 18 '16 at 18:14
0

Try this:

tests.sort(function(a, b) {
    var diff = a.title.localeCompare(b.title);
    return diff == 0 ? (a.modifiedDate ? a.modifiedDate.getTime() : Infinity) - (b.modifiedDate ? b.modifiedDate.getTime() : Infinity) : diff;
})
Will
  • 1,718
  • 3
  • 15
  • 23
  • 1
    You might even just do `return diff || (… - …);` – Bergi Mar 18 '16 at 14:17
  • I'd have upvoted this (nice solution!), but I think that `.modifiedDate` is a string (having a `.localeCompare` method) not a `Date` object, despite the name. – Bergi Mar 18 '16 at 14:18
  • @Bergi - I like the diff || (...) idea - thanks. It's cleaner. – Will Mar 18 '16 at 16:44