-
Notifications
You must be signed in to change notification settings - Fork 361
Decimal spec #5413
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
base: main
Are you sure you want to change the base?
Decimal spec #5413
Conversation
For comment in meeting invite around |
specs/decimal/decimal.md
Outdated
static DecimalValue FromUInt64(UInt64 value); | ||
static DecimalValue FromSingle(Single value); | ||
static DecimalValue FromDouble(Double value); | ||
static DecimalValue FromString(String value); // LCID=LOCALE_INVARIANT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For parsing, https://learn.microsoft.com/en-us/dotnet/api/system.decimal.tostring?view=net-9.0&redirectedfrom=MSDN#System_Decimal_ToString_System_String_ FromString has overloads for (string) and (string, IFormatProvider). That follows our usual rule of adding overload parameters after the base parameter list.
For formatting, C# goes the other way https://learn.microsoft.com/en-us/dotnet/api/system.decimal.tostring?view=net-9.0 and says that there's a four-way overload, with IFormatProvider being first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RECOMMEND:
- ToString()
- ToStringInvariant()
- ToStringWithLocale(string localeName)
RECOMMEND: DON'T overload ToString to avoid potential confusion by developers expecting parameter isn't a locale name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RECOMMEND: Ping Globalization owner about this
specs/decimal/decimal.md
Outdated
static DecimalValue FromUInt64(UInt64 value); | ||
static DecimalValue FromSingle(Single value); | ||
static DecimalValue FromDouble(Double value); | ||
static DecimalValue FromString(String value); // LCID=LOCALE_INVARIANT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For parsing, https://learn.microsoft.com/en-us/dotnet/api/system.decimal.tostring?view=net-9.0&redirectedfrom=MSDN#System_Decimal_ToString_System_String_ FromString has overloads for (string) and (string, IFormatProvider). That follows our usual rule of adding overload parameters after the base parameter list.
For formatting, C# goes the other way https://learn.microsoft.com/en-us/dotnet/api/system.decimal.tostring?view=net-9.0 and says that there's a four-way overload, with IFormatProvider being first.
@@ -45,6 +57,13 @@ data structure may be re-expressed in each language using language-specific cons | |||
defines the `DecimalValue` structure as the Win32 `DECIMAL` definition syntax is incompatible with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still suggest renaming DecimalValue
to just Decimal
now that the name is not used by the runtime class anymore.
Spec for decimal support