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

representation of decimal numbers #79

Closed
pvdbosch opened this issue Apr 28, 2021 · 7 comments
Closed

representation of decimal numbers #79

pvdbosch opened this issue Apr 28, 2021 · 7 comments
Milestone

Comments

@pvdbosch
Copy link
Contributor

pvdbosch commented Apr 28, 2021

How to represent decimal numbers with fixed precision: as number or as string type?

For use in MonetaryAmount, see belgif/openapi-money#2

For decimals with fixed precision:

So two options:

A) use number type with format, not in openapi-common

MoneyAmount:
  type: number
  format: decimal 

=> might be converted to float by default which is dangerous (can someone test?)

variant: add "multipleOf: 0.0001" to express precision. But this can also lead to other floating point problems for libraries implementing this (json-schema-org/json-schema-spec#312 , xeipuuv/gojsonschema#149).

B) define Decimal type based on string

  Decimal:
      type: string
      format: decimal # decimal is a custom format only supported in some tooling
      pattern: '^(\-|\+)?((\d+(\.\d*)?)|(\.\d+))$'  # based on BigDecimal (Java) but without exponent support. At least one digit required, before or after comma
      # valid values:  "100.234567", "010", "-.05", "+1", "10", "100." 
  UnsignedDecimal:
      type: string
      format: decimal # decimal is a custom format only supported in some tooling
      pattern: '^((\d+(\.\d*)?)|(\.\d+))$'  
      # valid values:  "100.234567", "010", "10", "100." but not "+1", "-1"

=> has to be converted in code from string to appropriate decimal type (e.g. BigDecimal in Java)

@pvdbosch pvdbosch added this to the backlog milestone Apr 28, 2021
@pvdbosch
Copy link
Contributor Author

The spec on number type: (JSON Schema refers to JSON RFC for this)

https://tools.ietf.org/html/rfc8259#section-6

  • This specification allows implementations to set limits on the range and precision of numbers accepted.
  • Since software that implements IEEE 754 binary64 (double precision) numbers is generally
    available and widely used, good interoperability can be achieved by implementations that expect no more precision or range than these provide
  • number type supports scientific notations like 5e-2 (=0.0.5)

@pvdbosch
Copy link
Contributor Author

pvdbosch commented Jan 12, 2022

update 13/1: add new results with newer swagger-codegen (the one in SwaggerHub), and tests with string type and format number

I did some tests with code generation:

  • type: number (without format or with decimal format) gets converted to:
    • decimal in C#
    • BigDecimal in Java
    • number/Number in typescript/javascript (which is like a Java double)
  • type: string, format: decimal gets converted to:
    • Java: BigDecimal using openapi-generator, String using swagger-codegen
    • typescript: string
    • couldn't test javascript (crashes)
  • type: string, format: number gets converted to:
    • Java: BigDecimal using both openapi-generator and swagger-codegen
    • typescript: Decimal (import from 3rd party decimal lib)
    • couldn't test javascript (crashes)

@pvdbosch
Copy link
Contributor Author

pvdbosch commented Jan 12, 2022

proposition:
document in REST guide, when precision is important, to use this:
type: string format: decimal
according to use case, a regexp pattern can be added to enforce digit pattern, number of digits before/after comma, ...

In openapi-money, for MonetaryAmount, use

      type: string
      format: decimal # decimal is a custom format only supported in some tooling
      pattern: '^(\-|\+)?((\d+(\.\d*)?)|(\.\d+))$'  # based on BigDecimal (Java) but without exponent support. At least one digit required, before or after comma
      # valid values:  "100.234567", "010", "-.05", "+1", "10", "100." 

I'll create a PR for validation next time.

@pvdbosch pvdbosch modified the milestones: backlog, in progress Jan 12, 2022
@pvdbosch
Copy link
Contributor Author

did some more tests with codegen tooling; updated the above comment with results.

Seems like type: string, format: number is better supported than decimal format so I'll go with that.

@pvdbosch
Copy link
Contributor Author

PR ready for review: #90
The beta version of openapi-money can also be reviewed: https://github.com/belgif/openapi-money/blob/master/src/main/openapi/money/v1beta/money-v1beta.yaml

@pvdbosch
Copy link
Contributor Author

pull request was review in WG of March, and merged

@pvdbosch
Copy link
Contributor Author

pvdbosch commented Apr 6, 2023

For future reference this page also sums up this problem space quite well: https://github.com/zalando/jackson-datatype-money/blob/main/MONEY.md
We took the "quoted decimal" option for MonetaryAmount and "fixed point" for EurocentAmount, prioritizing to avoid loss of precision over other downsides.

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

No branches or pull requests

1 participant