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

[API Proposal]: Scale property for the decimal datatype #65074

Closed
vanillajonathan opened this issue Feb 9, 2022 · 16 comments · Fixed by #66403
Closed

[API Proposal]: Scale property for the decimal datatype #65074

vanillajonathan opened this issue Feb 9, 2022 · 16 comments · Fixed by #66403
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime help wanted [up-for-grabs] Good issue for external contributors

Comments

@vanillajonathan
Copy link
Contributor

vanillajonathan commented Feb 9, 2022

Background and motivation

To quickly and easily get the number of decimal places of a decimal. Like SqlDecimal.

API Proposal

namespace System
{
    public struct Decimal // : ...
    {
+        // Gets the number of decimal places.
+        public byte Scale { get; }
    }
}

API Usage

decimal foo = 1.234m;
Console.WriteLine(foo.Scale); // 3

Alternative Designs

No response

Risks

No response

@vanillajonathan vanillajonathan added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 9, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 9, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Feb 9, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

To quickly and easily get the different parts (the whole number and the fractional part) of a decimal. Also allows to determine the length of the different parts.

API Proposal

namespace System
{
    public struct Decimal // : ...
    {
+        public int WholeNumber => //  The whole number part (before the decimal separator)
+        public int FractionalPart => // The decimal part (after the decimal separator)
    }
}

API Usage

decimal foo = 12.34m;
Console.WriteLine(foo.WholeNumber); // 12
Console.WriteLine(foo.FractionalPart); // 34
decimal foo = 1.2345m;
Console.WriteLine(foo.WholeNumber.ToString().Length); // 1
Console.WriteLine(foo.FractionalPart.ToString().Length); // 4

Alternative Designs

No response

Risks

No response

Author: vanillajonathan
Assignees: -
Labels:

api-suggestion, area-System.Runtime, untriaged

Milestone: -

@svick
Copy link
Contributor

svick commented Feb 9, 2022

  1. I think that this version of FractionalPart wouldn't be useful, because of leading zeroes: e.g. how would you differentiate between 1.5m and 1.05m, since presumably, both would return 5.
  2. You can already get the whole number part by casting to int. Keep in mind that the range of decimal is much larger than that of int, so this may throw. And this then also gives you a fairly simple way of getting the fractional part (as decimal): x - (int)x.
  3. Why do you think that getting the lengths of the two parts is something that should be easy? Is there some class of programs where this is a common operation?

@tannergooding
Copy link
Member

The signature as given wouldn't work/be useful, IMO.

There are cases where breaking it into (decimal IntegerPart, decimal FractionalPart) may be useful. This is used internally in the BCL and exposed by other languages as modf (or similar, based on the C/C++ name generally).

However, there are also limitations with how decimal can be used and what values it can represent and so it may not be as useful as it would be on some IEEE 754 decimal like type or the IEEE 754 binary types (float, double, half, etc)

@vanillajonathan
Copy link
Contributor Author

vanillajonathan commented Feb 10, 2022

@svick

  • I think that this version of FractionalPart wouldn't be useful, because of leading zeroes: e.g. how would you differentiate between 1.5m and 1.05m, since presumably, both would return 5.

  • You can already get the whole number part by casting to int. Keep in mind that the range of decimal is much larger than that of int, so this may throw. And this then also gives you a fairly simple way of getting the fractional part (as decimal): x - (int)x.

  • Why do you think that getting the lengths of the two parts is something that should be easy? Is there some class of programs where this is a common operation?

You're right, it wouldn't be useful as an integer as it could not differentiate between 1.5m and 1.05m, it would have to be some other kind of data type, maybe a string.

Yes, I can get the whole part easily by casting, but the decimal part is trickier. So I thought some properties could come in handy and be easier to use.

We have a web application where the users enter quantity (of invoice articles) and wanted to count the amount of decimals for data validation because a third-party API has a limit of three decimals.

@tannergooding

The signature as given wouldn't work/be useful, IMO.

I agree. It would have to be some other data type than int.

@svick
Copy link
Contributor

svick commented Feb 10, 2022

We have a web application where the users enter quantity (of invoice articles) and wanted to count the amount of decimals for data validation because a third-party API has a limit of three decimals.

I think an an efficient and still fairly clear way to do that is Math.Round(x, 3) == x. (Though you might also have to take care of trailing zeroes, since those are not considered by the == operator.)

@tannergooding
Copy link
Member

I'd agree with svick here. If all you're wanting to do is validation or ensuring user inputs meet the requirements, then there are much better ways to do that then trying to get the integer and fractional parts.

If you want to get something that's rounded to 3 digits, you have Math.Round. You can likewise trivially create your own string that's limited to 3 digits using ToString() and a relevant standard or custom numeric format string (ToString("F3") will give you exactly 3 fractional digits every time).

Given that this is decimal, you could also do something like Multiply by 1000, Truncate, and Divide by 1000 or one of many other options to get the right stuff.

If you do want to split into just integer/fractional parts; you can simply:

var integerPart = decimal.Truncate(x);
var fractionalPart = x - integerPart;

@vanillajonathan vanillajonathan changed the title [API Proposal]: FractionalPart property for the decimal datatype [API Proposal]: Scale property for the decimal datatype Feb 11, 2022
@vanillajonathan
Copy link
Contributor Author

I've rewritten the API proposal to instead provide a Scale property like SqlDecimal.

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Feb 11, 2022
@tannergooding
Copy link
Member

This seems like a reasonable thing to expose, uses the existing terminology from the existing constructor, and is a simple extract of the underlying data.

@GSPP
Copy link

GSPP commented Feb 16, 2022

Related:

Although this issue was closed, these proposed features still seem like a good idea to me.

@tannergooding
Copy link
Member

We had explicitly determined -0.0 was not a concept that should be exposed from decimal. It doesn't expose other key concepts like infinity or nan and so will never support things like transcedentals, etc.

Decimal is explicitly a "currency" type (and Currency probably would've been a better name, but hindsight is 20/20).

If we want to have concepts like these, then we are likely better off writing up a proposal and implementing the IEEE 754 decimal32, decimal64, and decimal128 types. These types are part of an industry standard, support the same concepts and operations as the binary floating-point types, and are generally more efficient to operate on and work with.

@terrajobst
Copy link
Member

terrajobst commented Mar 8, 2022

Video

  • Looks good as proposed
namespace System
{
    public partial struct Decimal
    {
        public byte Scale { get; }
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 8, 2022
@danmoseley
Copy link
Member

@vanillajonathan are you interested in offering a PR or should we mark up for grabs?

@vanillajonathan
Copy link
Contributor Author

@danmoseley Mark it as up for grabs.

@danmoseley danmoseley added the help wanted [up-for-grabs] Good issue for external contributors label Mar 9, 2022
@MichalPetryka
Copy link
Contributor

Could I be assigned to this? I'll do it now.

@tannergooding
Copy link
Member

Assigned out, thanks @MichalPetryka!

-- Noting we already discussed the needs/implementation here (new API, doc comment, and basic tests) in the C# Community Discord.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 9, 2022
MichalPetryka added a commit to MichalPetryka/runtime that referenced this issue Mar 9, 2022
Adds System.Decimal.Scale, a property that
returns the scaling factor of the decimal.

Closes dotnet#65074.
danmoseley pushed a commit that referenced this issue Mar 10, 2022
Adds System.Decimal.Scale, a property that
returns the scaling factor of the decimal.

Closes #65074.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants