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

Improve System.MidpointRounding documentation #19314

Closed
SingleAccretion opened this issue Jul 4, 2020 · 3 comments · Fixed by dotnet/dotnet-api-docs#4485
Closed

Improve System.MidpointRounding documentation #19314

SingleAccretion opened this issue Jul 4, 2020 · 3 comments · Fixed by dotnet/dotnet-api-docs#4485

Comments

@SingleAccretion
Copy link
Contributor

The problem

Rounding of floating point numbers is often considered one of the less intuitive .NET APIs. This makes good documentation in this area a necessity. Unfortunately, recent additions to the API surface of System.MidpointRounding do not enjoy the treatment of the thorough explanation that their existing counterparts have.

Right now, if I look at the table in the docs, these are the descriptions of the various rounding modes:

Mode Description
AwayFromZero When a number is halfway between two others, it is rounded toward the nearest number that is away from zero
ToEven When a number is halfway between two others, it is rounded toward the nearest even number
ToNegativeInfinity When a number is halfway between two others, it is rounded toward the result closest to and no greater than the infinitely precise result
ToPositiveInfinity When a number is halfway between two others, it is rounded toward the result closest to and no less than the infinitely precise result
ToZero When a number is halfway between two others, it is rounded toward the result closest to and no greater in magnitude than the infinitely precise result

There are a couple of problems with this table:

  1. Every sentence starts with "when a number is halfway between two others", implying that the rounding modes are all "round to nearest" (concerned with the 0.5 case). That is not the case - only the first two are, while the ones under them are "directed", always rounding to the values below/above them.
  2. The directed modes use wording that is, frankly speaking, quite cryptic. It talks about "the infinitely precise result", but what is the basis on which this result is calculated? What is the difference between "no greater" and "no greater in magnitude"? It seems to be concerned more about whether the result will be "a bit over" or "a bit under" than what it will actually be.
  3. There is nothing in the table that even suggests the first two modes are "round to nearest" while the other ones are "directed". Worse, it makes it seem like they are all "round to nearest" with different tie-breaking strategies.

This is my biggest problem with the doc, but it is not the only one. After the table there is a detailed section on the first two modes and what they do but there is nothing for the other modes.

This issue is a follow up to #38160.

Suggestions

My suggestions would be to:

  1. Change the inline docs on the modes to specifically call them out as "round to nearest" or "directed".
  2. Reorganize the doc into two sections for each group.
  3. Add the missing information on the "directed" modes.

I would like to contribute the improvements listed above, but there is a problem: I am not a domain expert. I can read the relevant part if the IEEE spec, but it is not a guarantee I will interpret it correctly. Is that acceptable given the history/sensitivity of the APIs in question?

@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label Jul 4, 2020
@gewarren gewarren added doc-enhancement and removed ⌚ Not Triaged Not triaged labels Jul 7, 2020
@gewarren
Copy link
Contributor

gewarren commented Jul 7, 2020

Here's the API doc: https://docs.microsoft.com/en-us/dotnet/api/system.midpointrounding?view=netcore-3.1.

@SingleAccretion If you want to work on this, that's great! After you submit a pull request, the technical experts will review it.

@SingleAccretion
Copy link
Contributor Author

@gewarren Great, I have started working on it.

@mikernet
Copy link

The enum summary should also be fixed as it currently says:

Specifies how mathematical rounding methods should process a number that is midway between two numbers.

Which has caused some confusion for users of my BigDecimal library.

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 a pull request may close this issue.

5 participants