10
\$\begingroup\$

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;

I'm looking for any feedback. Firstly, correctness, then performance.

Current 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]);
  }
}());
\$\endgroup\$
0

2 Answers 2

5
\$\begingroup\$
// 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];

As Flambino mentioned already it's probably not a good idea to store the actual digits of 253 in the source code. At least it's confusing. It's hard to tell if there's a typo. I would use a math function like Math.pow since it's so easy to read and does exactly what the comment says:

var lim = Math.pow(2, 53);
var limDigits = String(lim);

var safeIntegerStructure = new RegExp('^[-+]?0*(\\d{1,' + limLength + '})$');

I suggest using the RegExp constructor as little as possible because it's confusing. It's only necessary if you need to use dynamic expressions. Otherwise it adds much clutter from the escape sequences and string concatenations. In fact in this case you wanted to have a dynamic expression. But it's not really necessary. You can check the length of the capture group way easier.

var safeIntegerStructure = /^[-+]+0*(\d+)$/;

// If str is not a string, it cannot be a safe integer string.
if (Object.prototype.toString.call(str) !== '[object String]') {
  return false;
}

This looks pretty good. Depending on the use of the function it might make sense. And I have too little experience to argue with it. It's not even that important since I think <input> only ever has string values. Still you might consider to accept other types and coerce them to a string. I think it's pretty easy to read and to me it seems to be the usual approach in many libraries etc. when a function expects a string.

str = String(str);

Of course if you only want to accept strings this doesn't work. And since typeof str === 'string' doesn't work with String objects your way of using Object.prototype.toString looks like the best approach.


var match = str.match(safeIntegerStructure);

I've never used String#match this way. In my opinion the real purpose of this method is to get an array of all matches with a global regexp. For obtaining a single match I use RegExp#exec. But I suppose this might be just a personal preference since both versions have the exact same results.

var match = safeIntegerStructure.exec(str);

if (match === void 0) {
  return false;
}

I think this is a bug. Both String#match and RegExp#exec should return null when there is no match. But void anything is undefined. On the other hand when there is a match they return an array. I found checking the truth value to be the easiest to write and review.

if (!match) {
  return false;
}

var numerals = match[1];
var numeralsLength = numerals.length;
if (numeralsLength < limLength) {
  return true;
}
if (numeralsLength > limLength) {
  return false;
}
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;
  }
}

The for-loop iterates through the individual digits to compare them and stops at the first difference. Essentially this is exactly the same thing as the built-in string comparison operators do. So after making sure the entered number and the limit have the same number of digits you can compare them lexicographically:

var digits = match[1];
if (
  limDigits.length < digits.length ||
  limDigits.length === digits.length && limDigits < digits
) {
    return false;
}

Actually though it's a matter of opinion if the range of safe integers should be defined as [-253;+253] or [-(253-1);+(253-1)]. For example the ES6 draft defines Number.MAX_SAFE_INTEGER as 253-1. This makes sense considering that 253 already is ambiguous: Math.pow(2, 53) === Math.pow(2, 53) + 1

Using this alternative definition of "safe integer" you could safely use parseInt (or simple type coercion), removing the need for lexicographic comparison.

var lim = Math.pow(2, 53) - 1;
var value = +match[1];
if (lim < value) {
  return false;
}

So ultimately the function could become very short and therefore even easier to read in my opinion:

function isSafeInteger(str) {
  var match = /^[-+]?0*(\d+)$/.exec(String(str));
  return !!match && +match[1] < Math.pow(2, 53);
}
\$\endgroup\$
0
0
\$\begingroup\$

The current solution. See revision 6 of my question for the back story.

'use strict'

MAX_SAFE_INTEGER = Math.pow(2, 53) - 1
MIN_SAFE_INTEGER = -MAX_SAFE_INTEGER

SomeNamespace =
  tryParseSafeInteger: (string) ->
    match = ///
      ^            # The entire string must match
        \s*        # The number may have leading whitespace
        ([-+]?)    # The number may be signed (group 1)
        0*         # The number may have leading zeros
        (\d+)      # The number must contain at least one digit (group 2)
        (?:\.0*)?  # The number may have a fraction part equal to zero
        \s*        # The number may have trailing whitespace
      $
    ///.exec string

    return null unless match

    integer = parseInt match[1] + match[2], 10

    unless MIN_SAFE_INTEGER <= integer <= MAX_SAFE_INTEGER
      return null

    integer
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.