_.isInteger([1]) returns true

[JavaScriptDude]

There is an issue with the function predictates isInteger() and isFloat() where a number is passed in an array of length 1.

eg:

_.isInteger([1])
// true
_.isFloat([1.1])
// true
// both should be false.

The fix for this should be pretty easy by just adding if (i === undefined || i === null || i instanceof Array) return false to the top of isNumeric.

This simple gate will block undefined, null and array.

Suggestions?

[jgonggrijp]

@JavaScriptDude Thank you for reaching out. I agree the behavior is somewhat surprising. However, judging by the code comments and the use of parseFloat, I think the behavior is still as intended:

underscore-contrib/underscore.function.predicates.js

Lines 46 to 64 in 9eb0a98

	// A numeric is a variable that contains a numeric value, regardless its type
	// It can be a String containing a numeric value, exponential notation, or a Number object
	// See here for more discussion: http://stackoverflow.com/questions/18082/validate-numbers-in-javascript-isnumeric/1830844#1830844
	isNumeric: function(n) {
	return !isNaN(parseFloat(n)) && isFinite(n);
	},
	// An integer contains an optional minus sign to begin and only the digits 0-9
	// Objects that can be parsed that way are also considered ints, e.g. "123"
	// Floats that are mathematically equal to integers are considered integers, e.g. 1.0
	// See here for more discussion: http://stackoverflow.com/questions/1019515/javascript-test-for-an-integer
	isInteger: function(i) {
	return _.isNumeric(i) && i % 1 === 0;
	},
	// A float is a numbr that is not an integer.
	isFloat: function(n) {
	return _.isNumeric(n) && !_.isInteger(n);
	},

This is because [1].toString() returns "1", which meets the criteria for isInteger. Likewise for [1.1] and isFloat.

If this is not the behavior you want, you can define a new function that is closer to your liking. Perhaps like this:

function guardedIsInteger(value) {
    return (typeof value === 'number' || value instanceof Number) && _.isInteger(value);
}

If we do a 2.0 version at some point, we might redesign the behavior. Until that time, though, we should be careful not to break intended behavior. Other users may be relying on it.

[JavaScriptDude]

I strongly suggest 2.0 update. isNumeric(obj) should always return false for arrays.

FYI - reading the referenced stackoverflow post, the jQuery example works correctly as they check for array.

Thanks for your consideration.

[jgonggrijp]

@JavaScriptDude I must remind you that it is your opinion that isNumeric should behave in this particular way. Also, that jQuery is approaching a problem in some particular way, does not necessarily mean that other libraries should do that, too.

There will likely be a 2.0 at some point, but I'm going to hold off on that until Underscore 2.0 comes along. The behavior of isNumeric on arrays alone is not enough reason to hurry a major version bump.

[joshuacc]

Random drop-in from ye olde maintainer. I agree that this behavior is surprising and should probably be fixed.

Arguably it doesn't technically require a major version bump, since the documentation doesn't seem to indicate that arrays with one element are one of the types it intends to accept. Nevertheless, practically, I agree that waiting for 2.0 is best to avoid surprising breakage.

[JavaScriptDude]

One way to fix this without breaking would be to add a second parameter called strict. Under strict mode, we would check typeof and if not number return false. isInteger() and isFloat() would also get the strict parameter and pass it along to isNumeric().

[JavaScriptDude]

I'll be glad to pass on a PR if you would like. I already have a similar functionality working in my internal libraries.

[jgonggrijp]

It is perhaps a bit odd to have a strict parameter in an isX function, when none of the others have it, but you are right that it would be non-breaking. If you submit a PR, I will consider it.