Improve String.toInt and String.toFloat

[evancz]

With some parser work I’m doing, I wanted to expand the set of numbers that Elm can easily turn into Int and Float values.

Changes

• String.toFloat accepts scientific notation like 1e+40
• String.toInt accepts hex like 0xBEEF
• Added a bunch of tests for these things

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[brian-carroll]

If this is a number of minutes that I've extracted from a time-formatted string, like "09", it will fail. Is that desired behaviour?

[mbylstra]

Any chance you could also accept binary literals? eg 0b1010101 or 0B1010101. These are really helpful if you have to deal with parsing or generating binary file formats. Eg: If I just want the last 5 bits of a byte:

Bitwise.and myByte (String.toInt "0b11111")

With hex it's

Bitwise.and myByte 0x1F

which is far less readable. Parsing binary formats is really hard - this makes it a little easier!

[mbylstra]

Any chance you could also accept binary literals? eg 0b1010101 or 0B1010101. These are really helpful if you have to deal with parsing or generating binary file formats. Eg: If I just want the last 5 bits of a byte:

Bitwise.and myByte (String.toInt "0b11111")

With hex it's

Bitwise.and myByte 0x1F

which is far less readable. Parsing binary formats is really hard - this makes it a little easier!

[eeue56]

🎨 this is a good place for having fuzz tests, e.g

fuzz int "All ints are correctly converted to a string and back" (\int ->
  Expect.equal (Ok int) (String.toInt (toString int))

[eeue56]

💭 Why use this over isNaN? is it meant to be faster? If so, a comment might be worthwhile here since it's not obvious

[eeue56]

🎨 might be worth having a comment stating what the regex test does here (as it's not obvious unless you think about it/are familiar with non-decimal number strings)

[crazymykl]

Also, many languages (e. g. JavaScript, Ruby) allow capital letters for indicating base, 0XF and similar.

[justinmimbs]

The toInt function could be simplified with regexes. For example, the version below performs the same checks (but allows strings with leading zeros), and is a bit faster in some brief timing tests.

I think @brian-carroll brings up a good point. I would want "09" to parse without error. 👍

function toInt(s)
{
  // decimal
  if (/^[+-]?\d+$/.test(s))
  {
    return _elm_lang$core$Result$Ok(parseInt(s, 10));
  }

  // hex
  if (/^0[Xx][\dA-Fa-f]+$/.test(s))
  {
    return _elm_lang$core$Result$Ok(parseInt(s, 16));
  }

  return intErr(s);
}

(Edited hex regex to allow "0X" prefix, as pointed out by @crazymykl.)

[justinmimbs]

The toInt function could be simplified with regexes. For example, the version below performs the same checks (but allows strings with leading zeros), and is a bit faster in some brief timing tests.

I think @brian-carroll brings up a good point. I would want "09" to parse without error. 👍

function toInt(s)
{
  // decimal
  if (/^[+-]?\d+$/.test(s))
  {
    return _elm_lang$core$Result$Ok(parseInt(s, 10));
  }

  // hex
  if (/^0[Xx][\dA-Fa-f]+$/.test(s))
  {
    return _elm_lang$core$Result$Ok(parseInt(s, 16));
  }

  return intErr(s);
}

(Edited hex regex to allow "0X" prefix, as pointed out by @crazymykl.)

[mbylstra]

Something to keep in mind with parsing numbers like 09 is that a leading zero is often used as syntax for Octal representations. I'm not saying 09 should be parsed as an Octal number, it's just something to consider.

[mbylstra]

Something to keep in mind with parsing numbers like 09 is that a leading zero is often used as syntax for Octal representations. I'm not saying 09 should be parsed as an Octal number, it's just something to consider.

[evancz]

The alternate toInt implementation is not faster by my tests. The results reverse by swapping the order of the tests, so I suspect this has more to do with "VM warm up" than the actual details.

About parsing 0b1111, the code you write does not type check. toInt produces a Result. It'd be better to just allow binary digit literals, but that's a separate discussion that I do not want to have right now.

About leading zeros, I only know of JS making that octal, and it's getting phased out. I would prefer not to allow "too many" zeros at the front, but to be backwards compatible and release this as a core patch, I need to keep allowing it for now.

I've responded to other things with some code changes.

[evancz]

The alternate toInt implementation is not faster by my tests. The results reverse by swapping the order of the tests, so I suspect this has more to do with "VM warm up" than the actual details.

About parsing 0b1111, the code you write does not type check. toInt produces a Result. It'd be better to just allow binary digit literals, but that's a separate discussion that I do not want to have right now.

About leading zeros, I only know of JS making that octal, and it's getting phased out. I would prefer not to allow "too many" zeros at the front, but to be backwards compatible and release this as a core patch, I need to keep allowing it for now.

I've responded to other things with some code changes.

[evancz]

Thanks for the reviews everyone!

[evancz]

Thanks for the reviews everyone!