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

Add support for standard numeric format strings #788

Merged
merged 6 commits into from
May 28, 2020

Conversation

tmilnthorp
Copy link
Collaborator

@tmilnthorp tmilnthorp commented May 22, 2020

Add support for standard double numeric format strings. Also allow upper case format specifiers.

Fixes #786

@tmilnthorp
Copy link
Collaborator Author

@angularsen I suggest we deprecate the following format strings: "V", "v", "S", "s".

With this PR I think it makes more sense to use the standard format strings for these, or a custom string.Format on Value for fine-grained control.

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Merging #788 into master will increase coverage by 0.00%.
The diff coverage is 92.85%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #788   +/-   ##
=======================================
  Coverage   78.60%   78.60%           
=======================================
  Files         280      280           
  Lines       41834    41841    +7     
=======================================
+ Hits        32884    32890    +6     
- Misses       8950     8951    +1     
Impacted Files Coverage Δ
UnitsNet/QuantityFormatter.cs 94.44% <92.85%> (-2.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b36fd9...c5c20ef. Read the comment docs.

@angularsen
Copy link
Owner

@tmilnthorp Hm, I think maybe you are right. I have honestly never used the string format stuff much myself, but I can see how it doesn't really add much value to do myQuantity.ToString("v") rather than myQuantity.Value.ToString(). It's better to rely on the standard numeric formatting than reinventing our own.

@angularsen
Copy link
Owner

Getting late so I'll have to revisit this, but are we proposing to support standard numeric formats on our quantities? I mean, we could, but I have a feeling this is not commonly used or a best practice. Why can't the consumer just do $"{myLength.Value:f3} {myLength:a01}" to get 1.001 cm? Are there maybe use cases I am not remembering, that warrants supporting this many custom formats?

@lipchev
Copy link
Collaborator

lipchev commented May 22, 2020

Being in the category of people who are doing the dual string formatting- I would much prefer the simple "f3" format. My reasons are:

  1. Avoid MultiBinding for XAML
  2. "aX" is very much not standard - having it around in my code is both annoying and to some degree implies that I actually care about the abbreviation- when 99% of the time- I don't.
  3. If we somehow made the abbreviations dependent on a UnitSystem / RegionalSettings / CustomConfiguration- would I have to go over my existing code looking for those a02 ?

@tmilnthorp
Copy link
Collaborator Author

  • ToString should generally convey the information about the "whole" object. In our case, both the unit and the value are important in representing the entirety of the quantity.
  • In general, most string formatting cares about how the numeric part is formatted and less with the abbreviation (the default abbreviation is probably fine for the majority of use cases).
  • This also helps simplify formatting code from the client's perspective (using well-known formats) while also maintaining the flexibility to pass the individual parts to string.Format

@pgrawehr
Copy link
Contributor

Note that nothing prevents us from allowing $"{temperature:F3 v}" or $"{tempeature:g2 a1}", where you would get standard formatting and (optional) unit name in just one call. I think it is convenient to format one object in one call, as this prevents misstakes such as $"{temperature1:F3} {temperature2:v}".

@angularsen
Copy link
Owner

angularsen commented May 27, 2020

@tmilnthorp @pgrawehr Ok, I am convinced. I like the argument that ToString should generally convey the information about the "whole" object. and that the standard numeric formats simply only have effect on the value part. I also see the use case for WPF/WinUI and multi-binding.

@lipchev

"aX" is very much not standard - having it around in my code is both annoying and to some degree implies that I actually care about the abbreviation- when 99% of the time- I don't.

I forgot to mention, but a also works. It is the same as a0. If that was what you were referring to? If you meant that you didn't want to have two format specifiers in the string, then yes, I agree this proposal improves that.

@angularsen angularsen changed the title Add support for standard double numeric format strings. Also allow up… Add support for standard numeric format strings May 28, 2020
@angularsen angularsen merged commit c2ebeb9 into angularsen:master May 28, 2020
@angularsen
Copy link
Owner

Fixes #786

@angularsen
Copy link
Owner

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 this pull request may close these issues.

Quantity formatting: Trailing zeros
5 participants