-
Notifications
You must be signed in to change notification settings - Fork 393
QuantityInfo.MinValue and QuantityInfo.MaxValue are missing #848
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
Comments
I think this would be useful. Would you be interested in attempting a pull request? I'm happy to assist. |
A few notes:
|
It was rather simple to expose the But unfortunately I noticed that the current min/max values are not very useful since they always use the base unit. They neither represent a physical limit (e.g. light speed or absolute zero temperature) nor a lossless conversion limit of the datatype or the largest value that can be expressed by Units.Net. Regarding your notes: Since the Unless there are units where negative values are not allowed, I think both
But this depends of course on the intention of the I can create a PR if we can agree on an implementation. |
I agree.
Will this not cause problems? Length.MinValue.ToUnit(LengthUnit.Kilometer) // overflow I realize now that my own proposal of MinValue with min unit and MaxValue with max unit would have the same issue.
Different units have different conversion functions, so I think this is difficult to guarantee anyway.
How did you envision these to behave? |
@bitbonk and me use It is highly unlikely that any of our users will ever enter a value near the minimum or maximum and convert it to a different unit. So when using To solve our issue (avoid the usage of reflection to access MaxValue/MinValue), I could create a PR which just exposes these properties in But I would like to understand why There is also a potential problem with the decimal quantities. If you convert
Since we only compare the values using |
I think it was probably added without sufficient thought. I have never used it myself, and the more I think about it the more it seems like maybe we should not offer Min/Max for quantities. It is not intuitive what values or units to expect and you risk running into overflow issues. The benefit of keeping it is, that it encapsulates the complexity of what max value to safely choose for double vs decimal, and what unit to choose. The benefit of using BaseUnit as in the current implementation of Min/MaxValue is that it is less prone to run into overflow issues when comparing to other quantities, since it doesn't need to convert from a small/large unit to the base unit first. |
I would argue Min/Max should be removed entirely :) |
I have also struggled with the Min/Max in the past (before realizing the overflow issue was making them impossible to work with).
In summary, if we decide to keep the NumberGuard- I think there should be a safe way of checking input values such as to avoid the ArgumentException (imagine a method that is expected to divide one quantity by another- what sane check can one perform on the denominator? Using denominator > Zero is of course not 100% safe) |
There is also the issue of what do you do when the denominator is Zero- what reasonable result could be returned from such a method (other than the naturally resulting Nan/Infinity)? I am currently stuck with a bunch of nasty-pre-method-call-checks, for some calculated values- where the result could have simply been NaN/Infinity (imagine having a class with something like |
I tend to think it would be much cleaner and less error prone to introduce a dedicated construct (e.g. Nullable or similar) in your user code that deliberately expresses that a min or max coercion is not set, rather than using a quantity's MinValue or MaxValue for it. So I would agree with @tmilnthorp to remove any Min/Max values from Units.NET altogether. As it currently stands, MinValue and MaxValue would cause more confusion and create pitfalls than it would help. |
There is a use case for range checks, but I too would argue that this is best solved by the application. Either pick a small/large value or use nullability to identify unbounded min/max values. I think we more or less agree to remove Min/Max from the library then? I created a PR to deprecate them. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Is your feature request related to a problem? Please describe.
For a generic (WPF) UI where you can input any type of quantity, we need the ability to configure the maximum and minimum bounds that the user is currently allowed to enter for a given value. If the user can enter any value (unbounded), the bounds should be set to the minimum and maximum that the given quantity type generally allows. Unfortunately there is no generic way to determine the minimum and maximum of a given quantity type. Although all quantities have the concrete properties like
Pressure.MinValue
andPressure.MaxValue
, there is no generic/polymorph way to determine these values without knowing the quantity type at compile time.Describe the solution you'd like
Introduce the properties
QuantityInfo.MinValue
andQuantityInfo.MaxValue
, just like you have already introduced the properetyQuantityInfo.Zero
:Describe alternatives you've considered
We are currently using the following code:
The text was updated successfully, but these errors were encountered: