handle undefined order (#227)

* handle undefined during bisection

* avoid duplicate lo < hi test

* optimize compare1

* handle undefined during sort

* descending sort test

* better error for null accessor

* adopt default arguments

src/ascending.js

@@ -1,7 +1,3 @@
 export default function ascending(a, b) {
-  return a == null || b == null ? NaN
-    : a < b ? -1
-    : a > b ? 1
-    : a >= b ? 0
-    : NaN;
+  return a == null || b == null ? NaN : a < b ? -1 : a > b ? 1 : a >= b ? 0 : NaN;
 }

src/bisector.js

@@ -2,45 +2,43 @@ import ascending from "./ascending.js";
 
 export default function bisector(f) {
   let delta = f;
-  let compare = f;
+  let compare1 = f;
+  let compare2 = f;
 
   if (f.length === 1) {
     delta = (d, x) => f(d) - x;
-    compare = ascendingComparator(f);
+    compare1 = ascending;
+    compare2 = (d, x) => ascending(f(d), x);
   }
 
-  function left(a, x, lo, hi) {
-    if (lo == null) lo = 0;
-    if (hi == null) hi = a.length;
-    while (lo < hi) {
-      const mid = (lo + hi) >>> 1;
-      if (compare(a[mid], x) < 0) lo = mid + 1;
-      else hi = mid;
+  function left(a, x, lo = 0, hi = a.length) {
+    if (lo < hi) {
+      if (compare1(x, x) !== 0) return hi;
+      do {
+        const mid = (lo + hi) >>> 1;
+        if (compare2(a[mid], x) < 0) lo = mid + 1;
+        else hi = mid;
+      } while (lo < hi);
     }
     return lo;
   }
 
-  function right(a, x, lo, hi) {
-    if (lo == null) lo = 0;
-    if (hi == null) hi = a.length;
-    while (lo < hi) {
-      const mid = (lo + hi) >>> 1;
-      if (compare(a[mid], x) > 0) hi = mid;
-      else lo = mid + 1;
+  function right(a, x, lo = 0, hi = a.length) {
+    if (lo < hi) {
+      if (compare1(x, x) !== 0) return hi;
+      do {
+        const mid = (lo + hi) >>> 1;
+        if (compare2(a[mid], x) <= 0) lo = mid + 1;
+        else hi = mid;
+      } while (lo < hi);
     }
     return lo;
   }
 
-  function center(a, x, lo, hi) {
-    if (lo == null) lo = 0;
-    if (hi == null) hi = a.length;
+  function center(a, x, lo = 0, hi = a.length) {
     const i = left(a, x, lo, hi - 1);
     return i > lo && delta(a[i - 1], x) > -delta(a[i], x) ? i - 1 : i;
   }
 
   return {left, center, right};
 }
-
-function ascendingComparator(f) {
-  return (d, x) => ascending(f(d), x);
-}

src/quickselect.js

@@ -1,8 +1,10 @@
-import ascending from "./ascending.js";
+import {ascendingDefined, compareDefined} from "./sort.js";
 
 // Based on https://github.com/mourner/quickselect
 // ISC license, Copyright 2018 Vladimir Agafonkin.
-export default function quickselect(array, k, left = 0, right = array.length - 1, compare = ascending) {
+export default function quickselect(array, k, left = 0, right = array.length - 1, compare) {
+  compare = compare === undefined ? ascendingDefined : compareDefined(compare);
+
   while (right > left) {
     if (right - left > 600) {
       const n = right - left + 1;

src/sort.js

@@ -1,25 +1,37 @@
-import ascending from "./ascending.js";
 import permute from "./permute.js";
 
 export default function sort(values, ...F) {
   if (typeof values[Symbol.iterator] !== "function") throw new TypeError("values is not iterable");
   values = Array.from(values);
-  let [f = ascending] = F;
-  if (f.length === 1 || F.length > 1) {
+  let [f] = F;
+  if ((f && f.length === 1) || F.length > 1) {
     const index = Uint32Array.from(values, (d, i) => i);
     if (F.length > 1) {
       F = F.map(f => values.map(f));
       index.sort((i, j) => {
         for (const f of F) {
-          const c = ascending(f[i], f[j]);
+          const c = ascendingDefined(f[i], f[j]);
           if (c) return c;
         }
       });
     } else {
       f = values.map(f);
-      index.sort((i, j) => ascending(f[i], f[j]));
+      index.sort((i, j) => ascendingDefined(f[i], f[j]));
     }
     return permute(values, index);
   }
-  return values.sort(f);
+  return values.sort(f === undefined ? ascendingDefined : compareDefined(f));
+}
+
+export function compareDefined(compare) {
+  if (typeof compare !== "function") throw new TypeError("compare is not a function");
+  return (a, b) => {
+    const x = compare(a, b);
+    if (x || x === 0) return x;
+    return (compare(b, b) === 0) - (compare(a, a) === 0);
+  };
+}
+
+export function ascendingDefined(a, b) {
+  return (a == null || !(a >= a)) - (b == null || !(b >= b)) || (a < b ? -1 : a > b ? 1 : 0);
 }

test/bisect-test.js

@@ -141,3 +141,21 @@ it("bisectRight(array, value) handles large sparse d3", () => {
   assert.strictEqual(bisectRight(numbers, 5, i - 5, i), i - 0);
   assert.strictEqual(bisectRight(numbers, 6, i - 5, i), i - 0);
 });
+
+it("bisectLeft(array, value, lo, hi) keeps non-comparable values to the right", () => {
+  const values = [1, 2, null, undefined, NaN];
+  assert.strictEqual(bisectLeft(values, 1), 0);
+  assert.strictEqual(bisectLeft(values, 2), 1);
+  assert.strictEqual(bisectLeft(values, null), 5);
+  assert.strictEqual(bisectLeft(values, undefined), 5);
+  assert.strictEqual(bisectLeft(values, NaN), 5);
+});
+
+it("bisectRight(array, value, lo, hi) keeps non-comparable values to the right", () => {
+  const values = [1, 2, null, undefined];
+  assert.strictEqual(bisectRight(values, 1), 1);
+  assert.strictEqual(bisectRight(values, 2), 2);
+  assert.strictEqual(bisectRight(values, null), 4);
+  assert.strictEqual(bisectRight(values, undefined), 4);
+  assert.strictEqual(bisectRight(values, NaN), 4);
+});

test/sort-test.js

@@ -1,5 +1,5 @@
 import assert from "assert";
-import {descending, sort} from "../src/index.js";
+import {ascending, descending, sort} from "../src/index.js";
 
 it("sort(values) returns a sorted copy", () => {
   const input = [1, 3, 2, 5, 4];
@@ -12,6 +12,30 @@ it("sort(values) defaults to ascending, not lexicographic", () => {
   assert.deepStrictEqual(sort(input), [1, 2, "10"]);
 });
 
+// Per ECMAScript specification §23.1.3.27.1, undefined values are not passed to
+// the comparator; they are always put at the end of the sorted array.
+// https://262.ecma-international.org/12.0/#sec-sortcompare
+it("sort(values) puts non-orderable values last, followed by undefined", () => {
+  const date = new Date(NaN);
+  const input = [undefined, 1, null, 0, NaN, "10", date, 2];
+  assert.deepStrictEqual(sort(input), [0, 1, 2, "10", null, NaN, date, undefined]);
+});
+
+it("sort(values, comparator) puts non-orderable values last, followed by undefined", () => {
+  const date = new Date(NaN);
+  const input = [undefined, 1, null, 0, NaN, "10", date, 2];
+  assert.deepStrictEqual(sort(input, ascending), [0, 1, 2, "10", null, NaN, date, undefined]);
+  assert.deepStrictEqual(sort(input, descending), ["10", 2, 1, 0, null, NaN, date, undefined]);
+});
+
+// However we don't implement this spec when using an accessor
+it("sort(values, accessor) puts non-orderable values last", () => {
+  const date = new Date(NaN);
+  const input = [undefined, 1, null, 0, NaN, "10", date, 2];
+  assert.deepStrictEqual(sort(input, d => d), [0, 1, 2, "10", undefined, null, NaN, date]);
+  assert.deepStrictEqual(sort(input, d => d && -d), ["10", 2, 1, 0, undefined, null, NaN, date]);
+});
+
 it("sort(values, accessor) uses the specified accessor in natural order", () => {
   assert.deepStrictEqual(sort([1, 3, 2, 5, 4], d => d), [1, 2, 3, 4, 5]);
   assert.deepStrictEqual(sort([1, 3, 2, 5, 4], d => -d), [5, 4, 3, 2, 1]);
@@ -37,11 +61,12 @@ it("sort(values) accepts an iterable", () => {
 });
 
 it("sort(values) enforces that values is iterable", () => {
-  assert.throws(() => sort({}), TypeError);
+  assert.throws(() => sort({}), {name: "TypeError", message: "values is not iterable"});
 });
 
 it("sort(values, comparator) enforces that comparator is a function", () => {
-  assert.throws(() => sort([], {}), TypeError);
+  assert.throws(() => sort([], {}), {name: "TypeError", message: "compare is not a function"});
+  assert.throws(() => sort([], null), {name: "TypeError", message: "compare is not a function"});
 });
 
 it("sort(values) does not skip sparse elements", () => {