Fixed bug in _.sortedIndex that put undefined at the start (FIX: #1834)

[arianf]

Changed _.sortedIndex function to hangle undefined values correctly (FIXES: #1834 )

Revised previous pull request: #2041

Previously

if (iteratee(array[mid]) < value) low = mid + 1; else high = mid;

Changed it to:

var cur = iteratee(array[mid]);
if ( cur < value || (value === void 0 && cur <= Infinity) || (_.isNaN(value) && (cur <= Infinity || cur === void 0))) low = mid + 1; else high = mid;

The logic works because Undefined should be placed after the highest number possible + 1.

(iteratee(array[mid]) <= Inifinity)
// will always place undefined after the highest number, including Inifinity ( `<=` ).

Based on @jdalton request in #2041 (comment) I added NaN to always be placed after undefined.
Based off of #1768 (comment)

(_.isNaN(value) && (cur <= Infinity || cur === void 0)

The code aboves logic checks to see if the value is NaN, if it is go past Infinity or undefined.

_.sortedIndex([ 0, 10, 1000, undefined, NaN, NaN], undefined);
// => 3

_.sortedIndex([ 0, 10, 1000, undefined, NaN, NaN], NaN);
// => 4

So that how I built:

var cur = iteratee(array[mid]);
if ( cur < value || (value === void 0 && cur <= Infinity) || (_.isNaN(value) && (cur <= Infinity || cur === void 0))) low = mid + 1; else high = mid;

Since no numeric value can be greater than Infinity, it will place undefined values to the correct index.
And it will place all NaN past Infinity or undefined.

Also added edgeCaseTests, to verify it works:

test('sortedIndex conforms to sortBy for undefined (#1834)', function() {
    var sorted = _.sortBy([undefined, 1, undefined, 2]);
    equal(_.sortedIndex(sorted, undefined, _.identity), 2);

    var edgeCaseNumbers = [-Infinity, -Infinity, 0, Number.MAX_VALUE, Infinity, Infinity, undefined, undefined, NaN];

    var indexForUndefined = _.sortedIndex(edgeCaseNumbers, undefined);
    equal(indexForUndefined, 6, 'undefined should be inserted at index 6');

    var indexForNegInfinity = _.sortedIndex(edgeCaseNumbers, -Infinity);
    equal(indexForNegInfinity, 0, 'negative infinity should be inserted at index 0');

    var indexForInfinity = _.sortedIndex(edgeCaseNumbers, Infinity);
    equal(indexForInfinity, 4, 'infinity should be inserted at index 4');

    var indexForZero = _.sortedIndex(edgeCaseNumbers, 0);
    equal(indexForZero, 2, '0 should be inserted at index 2');

    var indexForNaN = _.sortedIndex(edgeCaseNumbers, NaN);
    equal(indexForNaN, 8, 'NaN should be inserted at index 8');

    var numbers = [10, 20, 30, 40, 50];

    var indexForUndefinedSimple = _.sortedIndex(numbers, undefined);
    equal(indexForUndefinedSimple, 5, 'undefined should be inserted at index 5');
  });

[megawac]

Number.MAX_VALUE won't exist in all environments. Other than that we'll have to discuss the implications. May be worth while to consider #1882 at the same time

[jdalton]

Number.MAX_VALUE is like ES3 or smth it should be supported everywhere.

[arianf]

I updated Number.MAX_VALUE to be just a really big number. What do you think of the rest of it?

[arianf]

@megawac To make it work with #1882 what if I change the signature to include a last argument?

_.sortedIndex = function(array, obj, iteratee, context, last)

where if last is set to true, it will return the last

[megawac]

I've been pretty busy the past couple weeks (exam time) but I'm playing with this logic now. I've almost got it passing (1 failing test) with this

if ( cur < value || (cur <= Infinity && !(value <= value)) ) low = mid + 1;
else high = mid;

[megawac]

Hows this look @jdalton https://github.com/jashkenas/underscore/compare/master...megawac:pr/2042?expand=1

(I don't recall why I stopped working on this branch)