Making compare() function more elegant

Question

I have this code:

function compare (a, b) {
  let comparison = 0;
  if (a.essentialsPercentage < b.essentialsPercentage) {
    comparison = 1;
  } else if (a.essentialsPercentage > b.essentialsPercentage) {
    comparison = -1;
  } else {
    if (a.skillsNicePercentage < b.skillsNicePercentage) {
      comparison = 1;
    } else if (a.skillsNicePercentage > b.skillsNicePercentage) {
      comparison = -1;
    } else {
      if (a.startDate > b.startDate) {
        comparison = 1
      } else if (a.startDate < b.startDate) {
        comparison = -1
      }
    }
  }
  return comparison;
}

What would be the most elegant way of writing it? It doesn't seems nice at the moment.

You can keep using `else if` rather than nesting `if` inside `else`. Other than that, it seems fine. — Barmar, Oct 29 '19 at 17:12

score 3 · Accepted Answer · answered Oct 29 '19 at 17:18

3

Assuming this is being used as the comparison function for Array.prototype.sort(), only the sign of the result matters, it doesn't have to be specifically -1 or 1. So instead of if and else, you can simply subtract the numbers.

compare(a, b) {
    let comparison = b.essentialPercentage - a.essentialPercentage;
    if (comparison == 0) {
        comparison = b.skillsNicePercentage - a.skillsNicePercentage;
        if (comparison == 0) {
            comparison = a.startDate - b.startDate;
        }
    }
    return comparison;
}

If any of the properties are strings rather than numbers, you can use localCompare instead of subtraction.

answered Oct 29 '19 at 17:18

Barmar

741,623
53
500
612

will it be as good without nesting or it is like that on purpose (what I suspect - otherwise you wouldn't rite it the way you did)? – jean d'arme Oct 29 '19 at 19:30
1

It will work without nesting as well, it might do an unnecessary test. – Barmar Oct 29 '19 at 19:45

georg · Answer 2 · 2019-10-29T17:23:34.787

2

This tiny function (or the equivalent <=> operator) is perhaps the most obvious lack in the js standard library:

// returns 1 if a > b, -1 if a < b, 0 if a == b
let cmp = (a, b) => (a > b) - (a < b)

Once you have defined it, chained comparisons are very easy:

compare = (a, b) =>
    cmp(a.essentialsPercentage, b.essentialsPercentage)
    || cmp(a.skillsNicePercentage, b.skillsNicePercentage)
    || cmp(a.startDate, b.startDate)

edited Oct 29 '19 at 17:23

answered Oct 29 '19 at 17:19

georg

211,518
52
313
390

The ordering of `startDate` is the reverse of the other two properties. – Barmar Oct 29 '19 at 17:20

score 0 · Answer 3 · answered Oct 29 '19 at 17:19

If you wanted to you could use a switch statement to keep each one of your cases nice and organized. This would be a "cleaner" method but not necessarily the most correct method. See this thread --> https://stackoverflow.com/a/2312837/11263228

Alternatively, you could create separate functions that will check each of your cases. For example, having a function that takes in a.skillsNicePercentage and b.skillsNicePercentage as parameters and returns true/false. This would be cleaner and also reusable!

Linh Nguyen · Answer 4 · 2019-10-29T17:41:21.860

0

Based on the logic in your comparison, this should work

function compare (a, b) {

  let comparison = a.skillsNicePercentage == b.skillsNicePercentage ? (a.startDate - b.startDate) : b.skillsNicePercentage - a.skillsNicePercentage
  let comparison1 = a.essentialsPercentage == b.essentialsPercentage ? b.skillsNicePercentage - a.skillsNicePercentage : comparison
  return comparison1;

}

edited Oct 29 '19 at 17:41

answered Oct 29 '19 at 17:28

Linh Nguyen

925
1
10
23

score 0 · Answer 5 · answered Oct 29 '19 at 17:39

You could generalize this in a simpler function that takes an array of field names and sorts the overall array based on those fields, in order. The code below takes some inspiration from the Mongoose database library and allows the - prefix on a field name to sort descending instead of the default ascending order. It also only works for numbers and strings; if you want to support other types, you'll have to extend the code.

function multiSort(fields) {
    return (a,b) => {
        let result = 0;
        for (let i = 0; result === 0 && i < fields.length; ++i) {
          let fieldName = fields[i];
          if (fieldName.charAt(0) === '-') {
              fieldName = fieldName.substring(1);
              if (typeof a[fieldName] === 'string') {
                  result = b[fieldName].localeCompare(a[fieldName]);
              }
              else {
                result = b[fieldName] - a[fieldName];
              }
          } else {
              if (typeof a[fieldName] === 'string') {
                  result = a[fieldName].localeCompare(b[fieldName]);
              }
              else {
                result = a[fieldName] - b[fieldName];
              }
          }
        }

        return result;
    };
}

This higher-order function will take an array of fields and return a function that sorts by those fields, in order, for strings and numbers, optionally with a field name prepended by - to sort that field in descending order. You'd use it like this:

someArrayValue.sort(multisort(['essentialsPercentage', 'skillsNicePercentage', '-startDate']));

Kamil Kiełczewski · Answer 6 · 2019-10-29T18:06:17.747

Try

function compare(a, b) {
  let c= b.essentialsPercentage - a.essentialsPercentage;
  let d= b.skillsNicePercentage - a.skillsNicePercentage;
  let e= a.startDate - b.startDate

  return Math.sign(c||d||e);
}

function compareNew(a, b) {
  let c= b.essentialsPercentage - a.essentialsPercentage;
  let d= b.skillsNicePercentage - a.skillsNicePercentage;
  let e= a.startDate - b.startDate

  return Math.sign(c||d||e);
}

function compareOld(a, b) {
  let comparison = 0;
  if (a.essentialsPercentage < b.essentialsPercentage) {
    comparison = 1;
  } else if (a.essentialsPercentage > b.essentialsPercentage) {
    comparison = -1;
  } else {
    if (a.skillsNicePercentage < b.skillsNicePercentage) {
      comparison = 1;
    } else if (a.skillsNicePercentage > b.skillsNicePercentage) {
      comparison = -1;
    } else {
      if (a.startDate > b.startDate) {
        comparison = 1
      } else if (a.startDate < b.startDate) {
        comparison = -1
      }
    }
  }
  return comparison;
}



// TESTS

a={essentialsPercentage:2,skillsNicePercentage:2,startDate:new Date(0)};
tests=[
  {essentialsPercentage:2,skillsNicePercentage:2,startDate:new Date(0)},
  {essentialsPercentage:2,skillsNicePercentage:2,startDate:new Date(+10000)},
  {essentialsPercentage:2,skillsNicePercentage:2,startDate:new Date(-10000)},
  {essentialsPercentage:2,skillsNicePercentage:2,startDate:new Date()},
  {essentialsPercentage:2,skillsNicePercentage:3,startDate:new Date()},
  {essentialsPercentage:2,skillsNicePercentage:1,startDate:new Date()},
  {essentialsPercentage:3,skillsNicePercentage:1,startDate:new Date()},  
  {essentialsPercentage:1,skillsNicePercentage:1,startDate:new Date()},    
]

tests.map(b=> console.log(`old: ${compareNew(a,b)}   new:${ compareOld(a,b)}`));

Making compare() function more elegant

6 Answers6