Update: See the end of the post for the latest version
Looking for any feedback. Firstly, correctness, then performance.
Background
I'm writing user input validation for a client-side JavaScript web application. Users enter numerals representing an integer into an <input>. As part of validation, the application must check that parseInt(input.value, 10) will result in the integer represented by the user's input.
In JavaScript, all numbers are floating-points. Within the interval [-253, 253], all integers have exact representations. The challenge is to guarantee that the user has provided a number within the interval. parseInt() doesn't work:
// Yields true
parseInt('9007199254740993', 10) === 9007199254740992;
Original Solution
var isSafeIntegerString = function() {
'use strict';
// The digits of the greatest and least safe integers, 2**53 and
// -2**53 respectively.
var limDigits = [9, 0, 0, 7, 1, 9, 9, 2, 5, 4, 7, 4, 0, 9, 9, 2];
// The number digits in the safe integer limits.
var limLength = limDigits.length;
// The structure of safe integers.
//
// ^...$ The entire string must match the structure
//
// [-+]? May be preceded by either a '-' or a '+'
//
// 0* May have leading zeros
//
// (\d{1, limLength}) Must contains between 1 and limLength digits
var safeIntegerStructure = new RegExp('^[-+]?0*(\\d{1,' + limLength + '})$');
return function(str) {
// If str is not a string, it cannot be a safe integer string.
if (Object.prototype.toString.call(str) !== '[object String]') {
return false;
}
// If str doesn't have the structure of a safe integer string, it
// can't be one.
var match = str.match(safeIntegerStructure);
if (match === void 0) {
return false;
}
// str has a safe integer structure. The RE captured the
// significant digits (i.e., without the sign or leading zeros).
var numerals = match[1];
var numeralsLength = numerals.length;
// If str contains less significant digits than the limit, it is
// less and therefore is safe.
if (numeralsLength < limLength) {
return true;
}
// If str contains more significant digits than the limit, it is
// greater and therefore is not safe.
if (numeralsLength > limLength) {
return false;
}
// str and the limit contain the same number of digits. So, compare
// each digit, starting from the most significant.
for (var i = 0; i < numeralsLength; ++i) {
var digit = parseInt(numerals.charAt(i), 10);
var limDigit = limDigits[i];
if (digit < limDigit) {
return true;
}
if (digit > limDigit) {
return false;
}
}
// str is equal to a safe integer limit.
return true;
};
}();
Tests
(function() {
var test = function(str, isSafe) {
console.log(str, isSafe, isSafeIntegerString(str));
};
var schemata = [
[ '0', true ],
[ '-0', true ],
[ '1', true ],
[ '-1', true ],
[ '900719925474099', true ],
[ '900719925474099', true ],
[ '9007199254740991', true ],
[ '9007199254740991', true ],
[ '9007199254740992', true ],
[ '-9007199254740992', true ],
[ '09007199254740992', true ],
['-09007199254740992', true ],
[ '9007199254740993', false],
[ '-9007199254740993', false],
];
for (var i = 0; i < schemata.length; ++i) {
var schema = schemata[i];
test(schema[0], schema[1]);
}
}());
Update 1: Reply to Robert
Hi Robert, thanks for the review.
You suggest obtaining the limit numerals by;
var limitNumerals = String(Math.pow(2, 53));
I didn't do this because I wasn't sure if String() would always use decimal notation (as opposed to scientific, e.g., String(100000000000000000000)). But, looking at the specification, I believe point 6 of 9.8.1 guarantees decimal notation.
Reviewer: 1 | My ego: 0
You suggest avoiding the RegExp constructor and checking the length of the captured numerals after:
var integerStructure = /^\s*[-+]?\s*0*(\d+)\s*$/;
Agreed this is cleaner. I've included the white-space stripe in the RE.
Reviewer: 2 | My ego: 0
You ask if coercing to a string would be better than doing a type check.
var str = String(obj);
Initial, I thought this would be unsafe but after trying loads of tests. I couldn't find any input that broke this. Only change it to avoid reassignment of an argument
Reviewer: 3 | My ego: 0
You suggest idiomatic use of REs.
var match = /^\s*[-+]?\s*0*(\d+)\s*$/.exec(str);
Seems to checkout. I'm just in-lining the RE.
Reviewer: 4 | My ego: 0
You found a bug:
if (!match) {
return false;
}
Yep.
Reviewer: 5 | My ego: -1
You suggest using lexicographical string comparison
var numerals = match[1];
if (numerals.length > limitNumerals.length ||
numerals.length === limitNumerals.length &&
numerals < limitNumerals) {
return false;
}
I believe that point 4 of 11.8.5 ensures this will work. I think code unit value refers to the Unicode code point, which means lexicographic sort does the right thing.
Reviewer: 6 | My ego: -1
So, that leaves us here:
var tryToSafeInteger = function(value) {
'use strict';
var str = String(value);
var match = /^\s*[-+]?\s*0*(\d+)\s*$/.exec(str);
if (!match) {
return null;
}
var numerals = match[1];
var numeralsLength = numerals.length;
var limitNumerals = String(Math.pow(2, 53));
var limitLength = limitNumerals.length;
if (numeralsLength > limitLength ||
(numeralsLength === limitLength && numerals > limitNumerals)) {
return null;
}
return parseInt(str, 10);
};
I've;
changed it to a failing conversion function since we're coercing instead of type checking; and
removed the closure (seems cleaner and modern JS engines I think would detect that they're constant).
Update 2
Further improvements following feedback from Robert:
Changed the function's name to help indicate that it's about turning a string into a number.
Only accept leading and trailing white space so
parseIntworks.Documented the RE and accept a zero-value fractional part.
Replaced the local cache of the string lengths with
.lengthto help emphasize thatnumeralsandlimitNumeralsare strings.Documented the lexicographic comparison.
Current version
var tryParseSafeInteger = function(value) {
'use strict';
var str = String(value);
// Match str if it represents an integer.
//
// ^...$ The entire string must match
//
// \s*..\s* The string may have leading and trailing white-space
//
// [-+]? The number may be signed
//
// 0* The number may have leading zeros
//
// \d+ The number must contain at least one digit
//
// (\.0*)? The number may have a fractional part equal to zero
//
var match = /^\s*[-+]?0*(\d+)(\.0*)?\s*$/.exec(str);
if (!match) {
return null;
}
var numerals = match[1];
var limitNumerals = String(Math.pow(2, 53));
// As numerals and limitNumerals only contain the numerals 0 to 9, if
// they are the same length, a lexicographic comparison is equivalent
// to the numeric comparison of the corresponding numbers.
if (numerals.length > limitNumerals.length ||
(numerals.length === limitNumerals.length && numerals > limitNumerals)) {
return null;
}
return parseInt(str, 10);
};
Update
I've been lying, slightly, the real implementation is in CoffeeScript. But the compiled JavaScript is the same as above
'use strict'
Forms.IntegerUtils =
tryParseSafeInteger: (value) ->
str = String value
match = ///
^ # The entire string must match
\s* # The number may have leading whitespace
[-+]? # The number may be signed
0* # The number may have leading zeros
(\d+) # The number must contain at least one digit
(\.0*)? # The number may have a fraction part equal to zero
\s* # The number may have trailing whitespace
$
///.exec str
return null unless match
numerals = match[1]
limitNumerals = String Math.pow 2, 53
# As numerals and limitNumerals only contain the numerals 0 to 9, if
# they are the same length, a lexicographic comparison is equivalent
# to the numeric comparison of the corresponding numbers.
if numerals.length > limitNumerals.length or \
numerals.length == limitNumerals.length and numerals > limitNumerals
return null
parseInt str, 10