Skip to content
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

Editorial: Improve the clarity and consistency of Number.prototype.toPrecision #2471

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gibson042
Copy link
Contributor

@gibson042 gibson042 commented Jul 24, 2021

We should strive to make algorithm steps maximally deterministic, and in particular should avoid interdependent assignment where possible.

1. Let _a_ be the first code unit of _m_.
1. Let _b_ be the other _p_ - 1 code units of _m_.
1. Let _a_ be the code unit at index 0 within _m_.
1. Let _b_ be the substring of _m_ from 1.
1. Set _m_ to the string-concatenation of _a_, *"."*, and _b_.
1. If _e_ > 0, then
1. Let _c_ be the code unit 0x002B (PLUS SIGN).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also thinking about improving the names of these aliases and replacing the code point references with inline literals.

Suggested change
1. Let _c_ be the code unit 0x002B (PLUS SIGN).
1. Let _sign_ be *"+"*.

@bakkot
Copy link
Contributor

bakkot commented Jul 24, 2021

This seems like an improvement to my eyes, although I'm not sure about the n < 10^p -> n ≤ 10^p change - what was the motivation for that?

Also, I note that there's a very similar text in Number.prototype.toExponential; presumably they should match.

@gibson042
Copy link
Contributor Author

This seems like an improvement to my eyes, although I'm not sure about the n < 10^p -> n ≤ 10^p change - what was the motivation for that?

This is actually the crux of the change, and it relates to cases where rounding increases magnitude. For example, consider x = 995 with p = 2. The old algorithm would require non-constructive discovery of (e=2, n=99) and (e=3, n=10), respectively corresponding to approximations 99e1 (990) and 10e2 (1000), and then tiebreaking to the latter because it is larger. The proposed algorithm instead anchors e=2 to discover n=99 and n=100 (the latter only allowed by looseness of "≤"), performs the same tiebreaking logic to settle on n=100, and then ends up at the same place by normalizing 100e1 to 10e2.

Also, I note that there's a very similar text in Number.prototype.toExponential; presumably they should match.

Yes, and also in ECMA-402 (this change was actually prompted by tc39/ecma402#572), and arguably in Number::toString as well. I'd like to explore the possibility of having them all defer to a new operation for finding the relevant integers, but haven't yet determined what shape it would have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants