Skip to content

Commit

Permalink
Fixes for non-orderable values.
Browse files Browse the repository at this point in the history
In addition to NaN, which is not equal to itself, you can have objects that are
not orderable due to defined valueOf functions which return NaN. For example:

  var o = new Number(NaN);

Here, o == o is true, but o <= o is false. Therefore it was possible for d3.min,
d3.max and d3.extent to observe these non-orderable values rather than ignore
them as intended. The fix is to check !(o <= o) rather than o == o.

This commit also fixes d3.bisect when the search value is non-orderable.
Previously, bisectLeft returned lo for non-orderable values, and bisectRight
returned hi. However, if the search value is non-orderable, then this return
value does not satisfy the conditions of bisection. This commit changes the
bisection methods to return NaN when the search value is non-orderable.

As a side-effect, the fix to d3.bisect now causes d3.scale.threshold to return
undefined when passed a non-orderable value, such as undefined. (Previously, a
threshold scale would return the highest value in the range, because it used
bisectRight internally.)
  • Loading branch information
mbostock committed Jun 18, 2013
1 parent 84f4a62 commit aad1b18
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 17 deletions.
14 changes: 8 additions & 6 deletions d3.js
Expand Up @@ -23,35 +23,35 @@ d3 = function() {
d3.min = function(array, f) {
var i = -1, n = array.length, a, b;
if (arguments.length === 1) {
while (++i < n && ((a = array[i]) == null || a != a)) a = undefined;
while (++i < n && ((a = array[i]) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = array[i]) != null && a > b) a = b;
} else {
while (++i < n && ((a = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null && a > b) a = b;
}
return a;
};
d3.max = function(array, f) {
var i = -1, n = array.length, a, b;
if (arguments.length === 1) {
while (++i < n && ((a = array[i]) == null || a != a)) a = undefined;
while (++i < n && ((a = array[i]) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = array[i]) != null && b > a) a = b;
} else {
while (++i < n && ((a = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null && b > a) a = b;
}
return a;
};
d3.extent = function(array, f) {
var i = -1, n = array.length, a, b, c;
if (arguments.length === 1) {
while (++i < n && ((a = c = array[i]) == null || a != a)) a = c = undefined;
while (++i < n && ((a = c = array[i]) == null || !(a <= a))) a = c = undefined;
while (++i < n) if ((b = array[i]) != null) {
if (a > b) a = b;
if (c < b) c = b;
}
} else {
while (++i < n && ((a = c = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = c = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null) {
if (a > b) a = b;
if (c < b) c = b;
Expand Down Expand Up @@ -92,6 +92,7 @@ d3 = function() {
d3.bisector = function(f) {
return {
left: function(a, x, lo, hi) {
if (!(x <= x)) return NaN;
if (arguments.length < 3) lo = 0;
if (arguments.length < 4) hi = a.length;
while (lo < hi) {
Expand All @@ -101,6 +102,7 @@ d3 = function() {
return lo;
},
right: function(a, x, lo, hi) {
if (!(x <= x)) return NaN;
if (arguments.length < 3) lo = 0;
if (arguments.length < 4) hi = a.length;
while (lo < hi) {
Expand Down
10 changes: 5 additions & 5 deletions d3.min.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/arrays/bisect.js
@@ -1,6 +1,7 @@
d3.bisector = function(f) {
return {
left: function(a, x, lo, hi) {
if (!(x <= x)) return NaN;
if (arguments.length < 3) lo = 0;
if (arguments.length < 4) hi = a.length;
while (lo < hi) {
Expand All @@ -11,6 +12,7 @@ d3.bisector = function(f) {
return lo;
},
right: function(a, x, lo, hi) {
if (!(x <= x)) return NaN;
if (arguments.length < 3) lo = 0;
if (arguments.length < 4) hi = a.length;
while (lo < hi) {
Expand Down
4 changes: 2 additions & 2 deletions src/arrays/extent.js
Expand Up @@ -5,13 +5,13 @@ d3.extent = function(array, f) {
b,
c;
if (arguments.length === 1) {
while (++i < n && ((a = c = array[i]) == null || a != a)) a = c = undefined;
while (++i < n && ((a = c = array[i]) == null || !(a <= a))) a = c = undefined;
while (++i < n) if ((b = array[i]) != null) {
if (a > b) a = b;
if (c < b) c = b;
}
} else {
while (++i < n && ((a = c = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = c = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null) {
if (a > b) a = b;
if (c < b) c = b;
Expand Down
4 changes: 2 additions & 2 deletions src/arrays/max.js
Expand Up @@ -4,10 +4,10 @@ d3.max = function(array, f) {
a,
b;
if (arguments.length === 1) {
while (++i < n && ((a = array[i]) == null || a != a)) a = undefined;
while (++i < n && ((a = array[i]) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = array[i]) != null && b > a) a = b;
} else {
while (++i < n && ((a = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null && b > a) a = b;
}
return a;
Expand Down
4 changes: 2 additions & 2 deletions src/arrays/min.js
Expand Up @@ -4,10 +4,10 @@ d3.min = function(array, f) {
a,
b;
if (arguments.length === 1) {
while (++i < n && ((a = array[i]) == null || a != a)) a = undefined;
while (++i < n && ((a = array[i]) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = array[i]) != null && a > b) a = b;
} else {
while (++i < n && ((a = f.call(array, array[i], i)) == null || a != a)) a = undefined;
while (++i < n && ((a = f.call(array, array[i], i)) == null || !(a <= a))) a = undefined;
while (++i < n) if ((b = f.call(array, array[i], i)) != null && a > b) a = b;
}
return a;
Expand Down
7 changes: 7 additions & 0 deletions test/arrays/bisect-test.js
Expand Up @@ -28,6 +28,13 @@ suite.addBatch({
assert.equal(bisect(array, 2.5), 2);
assert.equal(bisect(array, 3.5), 3);
},
"returns NaN if the search value is not orderable": function(bisect) {
var array = [1, 2, 3];
assert.isNaN(bisect(array));
assert.isNaN(bisect(array, undefined));
assert.isNaN(bisect(array, NaN));
assert.equal(bisect(array, null), 0);
},
"observes the optional lower bound": function(bisect) {
var array = [1, 2, 3, 4, 5];
assert.equal(bisect(array, 0, 2), 2);
Expand Down
3 changes: 3 additions & 0 deletions test/arrays/extent-test.js
Expand Up @@ -19,8 +19,11 @@ suite.addBatch({
assert.deepEqual(extent(["3", "20"]), ["20", "3"]);
},
"ignores null, undefined and NaN": function(extent) {
var o = {valueOf: function() { return NaN; }};
assert.deepEqual(extent([NaN, 1, 2, 3, 4, 5]), [1, 5]);
assert.deepEqual(extent([o, 1, 2, 3, 4, 5]), [1, 5]);
assert.deepEqual(extent([1, 2, 3, 4, 5, NaN]), [1, 5]);
assert.deepEqual(extent([1, 2, 3, 4, 5, o]), [1, 5]);
assert.deepEqual(extent([10, null, 3, undefined, 5, NaN]), [3, 10]);
assert.deepEqual(extent([-1, null, -3, undefined, -5, NaN]), [-5, -1]);
},
Expand Down
3 changes: 3 additions & 0 deletions test/arrays/max-test.js
Expand Up @@ -20,8 +20,11 @@ suite.addBatch({
assert.equal(max(["3", "20"]), "3");
},
"ignores null, undefined and NaN": function(max) {
var o = {valueOf: function() { return NaN; }};
assert.equal(max([NaN, 1, 2, 3, 4, 5]), 5);
assert.equal(max([o, 1, 2, 3, 4, 5]), 5);
assert.equal(max([1, 2, 3, 4, 5, NaN]), 5);
assert.equal(max([1, 2, 3, 4, 5, o]), 5);
assert.equal(max([10, null, 3, undefined, 5, NaN]), 10);
assert.equal(max([-1, null, -3, undefined, -5, NaN]), -1);
},
Expand Down
4 changes: 4 additions & 0 deletions test/arrays/min-test.js
Expand Up @@ -20,9 +20,13 @@ suite.addBatch({
assert.equal(min(["3", "20"]), "20");
},
"ignores null, undefined and NaN": function(min) {
var o = {valueOf: function() { return NaN; }};
assert.equal(min([NaN, 1, 2, 3, 4, 5]), 1);
assert.equal(min([o, 1, 2, 3, 4, 5]), 1);
assert.equal(min([1, 2, 3, 4, 5, NaN]), 1);
assert.equal(min([1, 2, 3, 4, 5, o]), 1);
assert.equal(min([10, null, 3, undefined, 5, NaN]), 3);
assert.equal(min([-1, null, -3, undefined, -5, NaN]), -5);
},
"compares heterogenous types as numbers": function(min) {
assert.strictEqual(min([20, "3"]), "3");
Expand Down
7 changes: 7 additions & 0 deletions test/scale/threshold-test.js
Expand Up @@ -26,6 +26,13 @@ suite.addBatch({
assert.equal(x(.8), "c");
assert.equal(x(1), "c");
},
"returns undefined if the specified value is not orderable": function(threshold) {
var x = threshold().domain([1/3, 2/3]).range(["a", "b", "c"]);
assert.isUndefined(x());
assert.isUndefined(x(undefined));
assert.isUndefined(x(NaN));
assert.equal(x(null), "a"); // null < 1/3
},
"domain values are arbitrary": function(threshold) {
var x = threshold().domain(["10", "2"]).range([0, 1, 2]);
assert.strictEqual(x.domain()[0], "10");
Expand Down

0 comments on commit aad1b18

Please sign in to comment.