Sorting array of objects by string vs numeric values behaving differently

Question

This question is a little long-winded, but please bear with me.

I have a general-purpose value-comparison function to use with .sort():

export function compareObjectValues(key: string, direction: string = 'asc') {
  // used as a comparator supplied to .sort() for sorting arrays of objects

  return function(a: Object, b: Object): number {
    const propertyA = getDescendantProperty(a, key);
    const propertyB = getDescendantProperty(b, key);

    if (!propertyA || !propertyB) {
      return 0;
    }


    const normalizedPropA = (typeof propertyA === 'string') ? (propertyA as string).toLocaleLowerCase() : propertyA;
    const normalizedPropB = (typeof propertyB === 'string') ? (propertyB as string).toLocaleLowerCase() : propertyB;
    let comparison = 0;

    if (normalizedPropA > normalizedPropB) {
      comparison = 1;
    }
    else {
      comparison = -1;
    }

    if (typeof normalizedPropA === 'number' && typeof normalizedPropB === 'number') {
      console.log('comparison', comparison);
      return comparison;
    }
    else {
      console.log('string comparison', comparison);
      return (direction === 'desc') ? (comparison * -1) : comparison;
    }
  };
}

The inner helper function, getDescendantProperty() is to access values on an object by dot path safely:

export function getDescendantProperty(obj: object, path: string) {
  // safely access nested properties if the property of the object is itself, an object

  if (path.includes('.')) {
    return path.split('.').reduce((accumulator: object, part: string) => accumulator && accumulator[part] || undefined, obj);
  }
  else {
    return obj[path];
  }
}

When I sort an array of objects using a string to sort by, compareObjectValues() works as expected. When using numeric values, it fails, and doesn't iterate through all of the expected values to compare.

Example:

describe('on an object that uses a number for sorting', () => {
  it('should return the comparison of the values in the correct order', () => {
    const objects = [
      { id: 4 },
      { id: 0 },
      { id: 2 },
      { id: 3 },
      { id: 1 },
    ];

    const sortedObjects = objects.sort(utils.compareObjectValues('id'));

    expect(sortedObjects).toEqual([{ id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }]);
  });
});

Karma output:

LOG: 'comparison', 1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 0 of 2 SUCCESS (0 secs / 0 secs)
LOG: 'comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 0 of 2 SUCCESS (0 secs / 0 secs)
LOG: 'comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 0 of 2 SUCCESS (0 secs / 0 secs)
Chrome 74.0.3729 (Mac OS X 10.14.5) Utils tests When calling `compareObjectValues` on an object that uses a number for sorting should return the comparison of the numbers in the correct order FAILED
        Error: Expected $[0].id = 4 to equal 0.
        Expected $[1].id = 0 to equal 1.
        Expected $[2].id = 1 to equal 2.
        Expected $[3].id = 2 to equal 3.
        Expected $[4].id = 3 to equal 4.
            at <Jasmine>
            at UserContext.<anonymous> (src/app/lib/utils.spec.ts:133:31)
            at ZoneDelegate../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (node_modules/zone.js/dist/zone.js:391:1)
            at ProxyZoneSpec.push../node_modules/zone.js/dist/zone-testing.js.ProxyZoneSpec.onInvoke (node_modules/zone.js/dist/zone-testing.js:308:1)
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
Chrome 74.0.3729 (Mac OS X 10.14.5) Utils tests When calling `compareObjectValues` on an object that uses a number for sorting should return the comparison of the numbers in the correct order FAILED
        Error: Expected $[0].id = 4 to equal 0.
        Expected $[1].id = 0 to equal 1.
        Expected $[2].id = 1 to equal 2.
        Expected $[3].id = 2 to equal 3.
        Expected $[4].id = 3 to equal 4.
            at <Jasmine>
            at UserContext.<anonymous> (src/app/lib/utils.spec.ts:133:31)
            at ZoneDelegate../node_modules/zone.js/dist/zone.js.ZoneDelegate.invoke (node_modules/zone.js/dist/zone.js:391:1)
LOG: 'string comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', 1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', 1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', 1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', -1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
LOG: 'string comparison', 1
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 1 of 2 (1 FAILED) (0 secs / 0.159 secs)
Chrome 74.0.3729 (Mac OS X 10.14.5): Executed 2 of 2 (1 FAILED) (0.19 secs / 0.16 secs)
TOTAL: 1 FAILED, 1 SUCCESS
TOTAL: 1 FAILED, 1 SUCCESS
npm ERR! Test failed.  See above for more details.

As you can see from the log, when processing the objects with numeric values, it stopped after 3 iterations. With string values, it processed 9.

I have verified that getDescendantProperty() returns the expected value. I have also verified that the logic of compareObjectValues() works as expected if I apply the same comparisons manually by passing a function to .sort() in my test.

I'm having some difficulty isolating my error and would greatly appreciate some help.

Answer 1

The problem lies in this lazy check for properties:

if (!propertyA || !propertyB) return 0;

This works fine for checking certain types of values, but will give you unexpected results when checking against numbers (!0 === true), empty strings (!'' === true) etc.

Use strict checks instead:

if (typeof propertyA === 'undefined' || typeof propertyB === 'undefined') return 0;