New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better `toString` implementation for `number` #731

Closed
Fresheyeball opened this Issue Oct 13, 2016 · 8 comments

Comments

Projects
None yet
7 participants
@Fresheyeball

Fresheyeball commented Oct 13, 2016

So here is the deally. I've now witnessed a scenario where an Elm variable of : number is used to set an inline style attribute. The problem is, is the number is large enough, return v + ''; will end up spitting the number out in scientific notation. For example

123300000000000000000000000 + '' = "1.233e+26"

so in Elm

toString 123300000000000000000000000 = "1.233e+26"

and CSS doesn't understand numbers formatted this way. For CSS to be happy, the number must be formatted without the e+. This SO post shows a way this can be done:

http://stackoverflow.com/questions/20001325/convert-big-number-to-string-without-scientific-notation

such that

toString 123300000000000000000000000 = "123300000000000000000000000"

which is what CSS needs. Atm I'll write a native module for the project to correct this. But I feel its the way it should be implemented in core, so the formatting is deterministic.

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Oct 13, 2016

Thanks for the issue! 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 commented Oct 13, 2016

Thanks for the issue! 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.

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Oct 14, 2016

Contributor

This is also true for very small values, |n| < 1e-6.

Contributor

mgold commented Oct 14, 2016

This is also true for very small values, |n| < 1e-6.

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Oct 14, 2016

Contributor

Generally speaking, I wonder whether it's better to accept that toString necessarily needs to make an arbitrary choice amongst various possible string representations (arbitrary in the sense that toString takes only the one parameter, so there is nothing you can do to vary the representation).

Basically, solving the CSS problem by changing toString feels like a strategy that can't be generalized. That is, having done it, how do you then solve the problem of the use case that needs the exponential notation?

So, it would possibly be better to have a new function, something like toCssString : number -> String. Another advantage of this is that the function name explicitly promises to return something useful for CSS.

Also, I think it could be implemented without a native module. At least, at first glance, there doesn't seem to be anything in the SO post you refer to that couldn't be implemented in Elm, given the current behaviour of toString.

Contributor

rgrempel commented Oct 14, 2016

Generally speaking, I wonder whether it's better to accept that toString necessarily needs to make an arbitrary choice amongst various possible string representations (arbitrary in the sense that toString takes only the one parameter, so there is nothing you can do to vary the representation).

Basically, solving the CSS problem by changing toString feels like a strategy that can't be generalized. That is, having done it, how do you then solve the problem of the use case that needs the exponential notation?

So, it would possibly be better to have a new function, something like toCssString : number -> String. Another advantage of this is that the function name explicitly promises to return something useful for CSS.

Also, I think it could be implemented without a native module. At least, at first glance, there doesn't seem to be anything in the SO post you refer to that couldn't be implemented in Elm, given the current behaviour of toString.

@Fresheyeball

This comment has been minimized.

Show comment
Hide comment
@Fresheyeball

Fresheyeball Oct 14, 2016

@rgrempel I agree toString needs to make an "arbitrary" choice amongst various possible representations. I just think it should make 1 choice, and be deterministic in how output is formatted. Its not just CSS that can suffer from this, but any code that depends on the 'standard' formatting of numbers to strings. I also don't see this as a problem for those who might need exponential notation.

toString : number -> String -- will guarantee to be always formatted in standard notation
toStringExponential : number -> String -- will always be exponential notation
toStringExponentIfOver : Int -> number -> String -- will return standard or exponential notation depending of if the length of the string exceeds an `Int`

The above would cover all the use cases without ambiguity. At the moment, the toString function is inconsistent in which format will be returned (potentially setting up code traps where code relying on the standard representation fails only in certain cases), and ambiguous (do you know the threshold at which point it will start with exponential notation off the top of your head? I think most developers would need to google for it).

Inconsistencies should not be the default, and when they do exist their presence should be explicit.

Fresheyeball commented Oct 14, 2016

@rgrempel I agree toString needs to make an "arbitrary" choice amongst various possible representations. I just think it should make 1 choice, and be deterministic in how output is formatted. Its not just CSS that can suffer from this, but any code that depends on the 'standard' formatting of numbers to strings. I also don't see this as a problem for those who might need exponential notation.

toString : number -> String -- will guarantee to be always formatted in standard notation
toStringExponential : number -> String -- will always be exponential notation
toStringExponentIfOver : Int -> number -> String -- will return standard or exponential notation depending of if the length of the string exceeds an `Int`

The above would cover all the use cases without ambiguity. At the moment, the toString function is inconsistent in which format will be returned (potentially setting up code traps where code relying on the standard representation fails only in certain cases), and ambiguous (do you know the threshold at which point it will start with exponential notation off the top of your head? I think most developers would need to google for it).

Inconsistencies should not be the default, and when they do exist their presence should be explicit.

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Oct 14, 2016

Contributor

That is a good point -- I think you are correct.

Although you might still want a toCssString in the mix, in order to promise to return something appropriate for CSS. (I suppose you could document that in toString, but toString is actually a -> String, so it's not entirely focused on numbers).

Contributor

rgrempel commented Oct 14, 2016

That is a good point -- I think you are correct.

Although you might still want a toCssString in the mix, in order to promise to return something appropriate for CSS. (I suppose you could document that in toString, but toString is actually a -> String, so it's not entirely focused on numbers).

@Kit-Ko

This comment has been minimized.

Show comment
Hide comment
@Kit-Ko

Kit-Ko Apr 5, 2017

I would also like to point out a strange behavior of toString that broke my API.

toString 8411167609000105751
"8411167609000106000" : String

Kit-Ko commented Apr 5, 2017

I would also like to point out a strange behavior of toString that broke my API.

toString 8411167609000105751
"8411167609000106000" : String

@OvermindDL1

This comment has been minimized.

Show comment
Hide comment
@OvermindDL1

OvermindDL1 Apr 5, 2017

I would also like to point out a strange behavior of toString that broke my API.

@Kit-Ko That is not a strange behaviour of toString, that is just how javascript 64-bit floating points work, when you encode the number 8411167609000105751 it gets encoded into the javascript float as 8.411167609000106e18, which when you print out becomes 8411167609000106000, which is entirely expected and is how floating point works. You'd need to define an actual integer type to handle that as you'd expect, which would slow down the code execution that uses them by a good amount and would likely be a backwards incompatible change in the Elm api/language.

32-bit numbers will be accurate (up to 54-bit if I recall correctly), but your number exceeds 32-bit, so expect oddness.

OvermindDL1 commented Apr 5, 2017

I would also like to point out a strange behavior of toString that broke my API.

@Kit-Ko That is not a strange behaviour of toString, that is just how javascript 64-bit floating points work, when you encode the number 8411167609000105751 it gets encoded into the javascript float as 8.411167609000106e18, which when you print out becomes 8411167609000106000, which is entirely expected and is how floating point works. You'd need to define an actual integer type to handle that as you'd expect, which would slow down the code execution that uses them by a good amount and would likely be a backwards incompatible change in the Elm api/language.

32-bit numbers will be accurate (up to 54-bit if I recall correctly), but your number exceeds 32-bit, so expect oddness.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 10, 2017

Member

In the upcoming version, this function is called Debug.toString and is not meant for use in production. You will have things like String.fromInt and String.fromFloat. I tend to think that when generating CSS values, there should be a custom function on top of that for formatting things as you want. I don't think that should be baked into core though.

Member

evancz commented Jul 10, 2017

In the upcoming version, this function is called Debug.toString and is not meant for use in production. You will have things like String.fromInt and String.fromFloat. I tend to think that when generating CSS values, there should be a custom function on top of that for formatting things as you want. I don't think that should be baked into core though.

@evancz evancz closed this Jul 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment