fix comparator bisector (d3#250)

* fix comparator bisector

* fix comparator bisector, again

Author: mbostock

src/bisector.js

@@ -1,14 +1,22 @@
 import ascending from "./ascending.js";
+import descending from "./descending.js";
 
 export default function bisector(f) {
-  let delta = f;
-  let compare1 = f;
-  let compare2 = f;
+  let compare1, compare2, delta;
 
+  // If an accessor is specified, promote it to a comparator. In this case we
+  // can test whether the search value is (self-) comparable. We can’t do this
+  // for a comparator (except for specific, known comparators) because we can’t
+  // tell if the comparator is symmetric, and an asymmetric comparator can’t be
+  // used to test whether a single value is comparable.
   if (f.length !== 2) {
-    delta = (d, x) => f(d) - x;
     compare1 = ascending;
     compare2 = (d, x) => ascending(f(d), x);
+    delta = (d, x) => f(d) - x;
+  } else {
+    compare1 = f === ascending || f === descending ? f : zero;
+    compare2 = f;
+    delta = f;
   }
 
   function left(a, x, lo = 0, hi = a.length) {
@@ -42,3 +50,7 @@ export default function bisector(f) {
 
   return {left, center, right};
 }
+
+function zero() {
+  return 0;
+}

test/bisect-test.js

@@ -151,6 +151,12 @@ it("bisectLeft(array, value, lo, hi) keeps non-comparable values to the right",
   assert.strictEqual(bisectLeft(values, NaN), 5);
 });
 
+it("bisectLeft(array, value, lo, hi) keeps comparable values to the left", () => {
+  const values = [null, undefined, NaN];
+  assert.strictEqual(bisectLeft(values, 1), 0);
+  assert.strictEqual(bisectLeft(values, 2), 0);
+});
+
 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);

test/bisector-test.js

@@ -151,6 +151,37 @@ it("bisector(comparator).right(array, value) handles large sparse d3", () => {
   assert.strictEqual(bisectRight(boxes, box(6), i - 5, i), i - 0);
 });
 
+it("bisector(comparator).left(array, value) supports an asymmetric (object, value) comparator", () => {
+  const boxes = [1, 2, 3].map(box);
+  const bisectLeft = bisector(ascendingBoxValue).left;
+  assert.strictEqual(bisectLeft(boxes, 1), 0);
+  assert.strictEqual(bisectLeft(boxes, 2), 1);
+  assert.strictEqual(bisectLeft(boxes, 3), 2);
+});
+
+// This is not possible because the bisector has no way of knowing whether the
+// given comparator is symmetric or asymmetric, and if the comparator is
+// asymmetric it cannot be used to test the search value for orderability.
+it.skip("bisector(comparator).left(array, value) keeps non-comparable values to the right", () => {
+  const boxes = [1, 2, null, undefined, NaN].map(box);
+  const bisectLeft = bisector(ascendingBox).left;
+  assert.strictEqual(bisectLeft(boxes, box(1)), 0);
+  assert.strictEqual(bisectLeft(boxes, box(2)), 1);
+  assert.strictEqual(bisectLeft(boxes, box(null)), 5);
+  assert.strictEqual(bisectLeft(boxes, box(undefined)), 5);
+  assert.strictEqual(bisectLeft(boxes, box(NaN)), 5);
+});
+
+it("bisector(accessor).left(array, value) keeps non-comparable values to the right", () => {
+  const boxes = [1, 2, null, undefined, NaN].map(box);
+  const bisectLeft = bisector(unbox).left;
+  assert.strictEqual(bisectLeft(boxes, 1), 0);
+  assert.strictEqual(bisectLeft(boxes, 2), 1);
+  assert.strictEqual(bisectLeft(boxes, null), 5);
+  assert.strictEqual(bisectLeft(boxes, undefined), 5);
+  assert.strictEqual(bisectLeft(boxes, NaN), 5);
+});
+
 it("bisector(accessor).left(array, value) returns the index of an exact match", () => {
   const boxes = [1, 2, 3].map(box);
   const bisectLeft = bisector(unbox).left;
@@ -346,3 +377,7 @@ function unbox(box) {
 function ascendingBox(a, b) {
   return ascending(a.value, b.value);
 }
+
+function ascendingBoxValue(a, value) {
+  return ascending(a.value, value);
+}