Skip to content

May review #108

@waldemarhorwat

Description

@waldemarhorwat

Here's my review so far. I like the direction and approach. A number of problems remain with rounding which will make users believe that Amount can't be used to get reliably rounded results. Also, parts of the spec contradict itself and the explainer, and I'm not sure what the intent there is.

  • My main observation from last meeting still stands: whatever behavior we standardize in the first released version should be the behavior we're happy with long-term, including for things like converting 2m to person-height in the US or France. We shouldn't make users set a flag in the future to get the good result rather than the one we standardized first. Releasing this simultaneously with Intl-Sequence-Units is one possible solution.
  • The explainer and spec contradict each other in the convertTo API. The explainer has unit and usage options, the spec does not.
  • The spec's note in the Amount constructor is incorrect: The spec always stores Number and BigInt arguments directly in [[AmountValue]], regardless of the rounding options.
  • Exponents of 0 are not preserved when handing a string such as "0e-4". I'm happy with that behavior (and wouldn't want to change it) but wanted to make sure that's clear to everyone.
  • Rounding continues to produce surprises:
    • Converting 5 grams to tonnes results in 0.0000049999999999999996 tonnes instead of 0.000005 tonnes. This is caused by double rounding when doing the conversion. There are techniques to do the conversion with just a single rounding at the end which would produce the exactly rounded result as long as the conversion factors aren't too large (i.e. we're not converting solar-masses to daltons).
    • Converting "0.15" meters to meters with conversion options fractionDigits=1 and the default rounding mode "halfEven" produces "1e-1" meters, which is incorrect. The correct answer is "2e-1" meters. Again, multiple rounding is the culprit here.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions