Don’t consider null comparable. (#210)

* greatest, least, greatestIndex and leastIndex ignore nulls

closes #208

* test null, undefined

* don’t consider null comparable

* Update README

Co-authored-by: Philippe Rivière <fil@rezo.net>

README.md

@@ -302,19 +302,19 @@ Returns -1 if *a* is less than *b*, or 1 if *a* is greater than *b*, or 0. This
 
 ```js
 function ascending(a, b) {
-  return 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;
 }
 ```
 
 Note that if no comparator function is specified to the built-in sort method, the default order is lexicographic (alphabetical), not natural! This can lead to surprising behavior when sorting an array of numbers.
 
 <a name="descending" href="#descending">#</a> d3.<b>descending</b>(<i>a</i>, <i>b</i>) · [Source](https://github.com/d3/d3-array/blob/master/src/descending.js), [Examples](https://observablehq.com/@d3/d3-ascending)
 
-Returns -1 if *a* is greater than *b*, or 1 if *a* is less than *b*, or 0. This is the comparator function for reverse natural order, and can be used in conjunction with the built-in array sort method to arrange elements in descending order.  It is implemented as:
+Returns -1 if *a* is greater than *b*, or 1 if *a* is less than *b*, or 0. This is the comparator function for reverse natural order, and can be used in conjunction with the built-in array sort method to arrange elements in descending order. It is implemented as:
 
 ```js
 function descending(a, b) {
-  return b < a ? -1 : b > a ? 1 : b >= a ? 0 : NaN;
+  return a == null || b == null ? NaN : b < a ? -1 : b > a ? 1 : b >= a ? 0 : NaN;
 }
 ```
 

src/ascending.js

@@ -1,3 +1,7 @@
 export default function(a, b) {
-  return 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/descending.js

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

test/greatest-test.js

@@ -46,6 +46,14 @@ it("greatest(array) returns the first of equal values", () => {
   assert.deepStrictEqual(greatest([3, 2, 2, 1, 1, 0, 0, 0, 3, 0].map(box), ascendingValue), {value: 3, index: 0});
 });
 
+it("greatest(array) ignores null and undefined", () => {
+  assert.deepStrictEqual(greatest([null, -2, undefined]), -2);
+});
+
+it("greatest(array, accessor) ignores null and undefined", () => {
+  assert.deepStrictEqual(greatest([null, -2, undefined], d => d), -2);
+});
+
 function box(value, index) {
   return {value, index};
 }

test/greatestIndex-test.js

@@ -45,3 +45,11 @@ it("greatestIndex(array) returns the first of equal values", () => {
   assert.strictEqual(greatestIndex([-2, -2, -1, -1, 0, 0, 0, -3, 0]), 4);
   assert.strictEqual(greatestIndex([-3, -2, -2, -1, -1, 0, 0, 0, -3, 0], descending), 0);
 });
+
+it("greatestIndex(array) ignores null and undefined", () => {
+  assert.deepStrictEqual(greatestIndex([null, -2, undefined]), 1);
+});
+
+it("greatestIndex(array, accessor) ignores null and undefined", () => {
+  assert.deepStrictEqual(greatestIndex([null, -2, undefined], d => d), 1);
+});

test/least-test.js

@@ -46,6 +46,14 @@ it("least(array) returns the first of equal values", () => {
   assert.deepStrictEqual(least([3, 2, 2, 1, 1, 0, 0, 0, 3, 0].map(box), descendingValue), {value: 3, index: 0});
 });
 
+it("least(array) ignores null and undefined", () => {
+  assert.deepStrictEqual(least([null, 2, undefined]), 2);
+});
+
+it("least(array, accessor) ignores null and undefined", () => {
+  assert.deepStrictEqual(least([null, 2, undefined], d => d), 2);
+});
+
 function box(value, index) {
   return {value, index};
 }

test/leastIndex-test.js

@@ -45,3 +45,11 @@ it("leastIndex(array) returns the first of equal values", () => {
   assert.strictEqual(leastIndex([2, 2, 1, 1, 0, 0, 0, 3, 0]), 4);
   assert.strictEqual(leastIndex([3, 2, 2, 1, 1, 0, 0, 0, 3, 0], descending), 0);
 });
+
+it("leastIndex(array) ignores null and undefined", () => {
+  assert.deepStrictEqual(leastIndex([null, 2, undefined]), 1);
+});
+
+it("leastIndex(array, accessor) ignores null and undefined", () => {
+  assert.deepStrictEqual(leastIndex([null, 2, undefined], d => d), 1);
+});