1
\$\begingroup\$

I'm trying to sort an array with JavaScript. The objects should be sorted based on several properties (about 4 in my real scenario), which are all strings that may also be null or undefined. A null or undefined value should be treated similar, and should be "earlier" in the sorted array than items with values.

I used unit tests to spec the behavior, and the simplest code that would work (which is really, really verbose) is the following:

function myComparer(a, b) {
  if (!!a.label && !!b.label) {
    var diff = a.label.localeCompare(b.label);
    if (diff !== 0) {
      return diff;
    }
  }
  
  if (!!b.label && !a.label) {
    return -1;
  }
  
  if (!!a.label && !b.label) {
    return 1;
  }
  
  if (!!a.code && !!b.code) {
    var diff = a.code.localeCompare(b.code);
    if (diff !== 0) {
      return diff;
    }
  }
  
  if (!!b.code && !a.code) {
    return -1;
  }
  
  if (!!a.code && !b.code) {
    return 1;
  }
  
  return 0;
}

test("Can compare given labels", function() {
  var result = [{label: 'Z'}, {label: 'A'}].sort(myComparer);
  strictEqual(result[0].label, 'A');
  strictEqual(result[1].label, 'Z');
});

test("Can compare null label with given label", function() {
  var result = [{label: null}, {label: 'A'}].sort(myComparer);
  strictEqual(result[0].label, null);
  strictEqual(result[1].label, 'A');
});

test("Can compare null label with given label - reverse", function() {
  var result = [{label: 'Z'}, {label: null}].sort(myComparer);
  strictEqual(result[0].label, null);
  strictEqual(result[1].label, 'Z');
});

test("Can compare given codes", function() {
  var result = [{code: 'Z'}, {code: 'A'}].sort(myComparer);
  strictEqual(result[0].code, 'A');
  strictEqual(result[1].code, 'Z');
});

test("Can compare null code with given code", function() {
  var result = [{code: null}, {code: 'A'}].sort(myComparer);
  strictEqual(result[0].code, null);
  strictEqual(result[1].code, 'A');
});

test("Can compare null code with given code - reverse", function() {
  var result = [{code: 'Z'}, {code: null}].sort(myComparer);
  strictEqual(result[0].code, null);
  strictEqual(result[1].code, 'Z');
});

test("Will sort on label before code", function() {
  var result = [
    {id: 1, label: 'Z', code: 'Z'},
    {id: 2, label: 'Z', code: 'A'},
    {id: 3, label: 'A', code: 'Z'},
    {id: 4, label: 'A', code: 'A'}].sort(myComparer);
  strictEqual(result[0].id, 4);
  strictEqual(result[1].id, 3);
  strictEqual(result[2].id, 2);
  strictEqual(result[3].id, 1);
  
});
<script src="http://code.jquery.com/qunit/qunit-1.12.0.js"></script>
<link href="http://code.jquery.com/qunit/qunit-1.12.0.css" rel="stylesheet"/>
<div id="qunit"></div>
<div id="qunit-fixture"></div>

This will not be pretty for 4 or more properties (I figured you can imagine this, I tried to keep my example a bit smaller).

I can think of several ways to improve this, but I struggle to balance:

  • Performance
  • Readability
  • Verbosity

Which is why I figured I'd ask here: do you have any suggestions on how to refactor my version into something more elegant?

PS. I'm happy to use ECMAScript5 stuff, I can use polyfills if anything's missing in browsers.

\$\endgroup\$
8
\$\begingroup\$

I saw this because you linked to it from a Knockout-related question on Stack Overflow. My first suggestion is to create a function that compares a single property, since the comparison of each property is the same. You can also use some other shortcuts.

function compareProperty(a, b) {
  return (a || b) ? (!a ? -1 : !b ? 1 : a.localeCompare(b)) : 0;
}

function myComparer(a, b) {
  return compareProperty(a.label, b.label) || compareProperty(a.code, b.code);
}

function compareProperty(a, b) {
  return (a || b) ? (!a ? -1 : !b ? 1 : a.localeCompare(b)) : 0;
}

function myComparer(a, b) {
  return compareProperty(a.label, b.label) || compareProperty(a.code, b.code);
}




test("Can compare given labels", function() {
  var result = [{label: 'Z'}, {label: 'A'}].sort(myComparer);
  strictEqual(result[0].label, 'A');
  strictEqual(result[1].label, 'Z');
});

test("Can compare null label with given label", function() {
  var result = [{label: null}, {label: 'A'}].sort(myComparer);
  strictEqual(result[0].label, null);
  strictEqual(result[1].label, 'A');
});

test("Can compare null label with given label - reverse", function() {
  var result = [{label: 'Z'}, {label: null}].sort(myComparer);
  strictEqual(result[0].label, null);
  strictEqual(result[1].label, 'Z');
});

test("Can compare given codes", function() {
  var result = [{code: 'Z'}, {code: 'A'}].sort(myComparer);
  strictEqual(result[0].code, 'A');
  strictEqual(result[1].code, 'Z');
});

test("Can compare null code with given code", function() {
  var result = [{code: null}, {code: 'A'}].sort(myComparer);
  strictEqual(result[0].code, null);
  strictEqual(result[1].code, 'A');
});

test("Can compare null code with given code - reverse", function() {
  var result = [{code: 'Z'}, {code: null}].sort(myComparer);
  strictEqual(result[0].code, null);
  strictEqual(result[1].code, 'Z');
});

test("Will sort on label before code", function() {
  var result = [
    {id: 1, label: 'Z', code: 'Z'},
    {id: 2, label: 'Z', code: 'A'},
    {id: 3, label: 'A', code: 'Z'},
    {id: 4, label: 'A', code: 'A'}].sort(myComparer);
  strictEqual(result[0].id, 4);
  strictEqual(result[1].id, 3);
  strictEqual(result[2].id, 2);
  strictEqual(result[3].id, 1);
  
});
<script src="http://code.jquery.com/qunit/qunit-1.12.0.js"></script>
<link href="http://code.jquery.com/qunit/qunit-1.12.0.css" rel="stylesheet"/>
<div id="qunit"></div>
<div id="qunit-fixture"></div>

Using the conditional-expression operator greatly shortens the code, but the biggest gain comes from using a function for common code. I also rearranged the conditional logic so that it doesn't have to make as many comparisons.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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