Concern about UnitsNet NumberExtensions - Convert Anything to Double

[steelxjoel]

My concern relates to how easy it is to make a mistake with the NumberExtensions in C#, due to the fact that the implementation will allow 'attempt to convert any object to a UnitsNet type'... including other UnitsNet types.

I nuget'ed the NumberExtensions package because I loved the idea of doing stuff like:
var gravity = 9.8.MetersPerSecondSquared()

After some slow introduction of the UnitsNet and NumberExtensions to a small part of our larger engineering codebase and testing it, I found that I had already made an error. Something along the lines of
var myCalc = someNumber * myLength.Millimeters()

'myLength' was originally initialised from meters, and had a value of 0.14(m). Then, myLength.Millimeters() casts the value to a double and then creates a new Length type with the value of 0.14mm. <-- which is of course a value 3 significant digits distant from the original!

It was a mistake on my part; what I actually needed was to just use myLength.Millimeters (without the parentheses). At first I slapped my own head and fixed the code, but then I realised: this is such a simple mistake that will be made again. At this point I feel it would be irresponsible for me to continue rolling out the package within our codebase without addressing this.

My solution is to abolish the generic conversion methods that attempt casts from any type, and instead only support extension methods for double and decimal. Such as

`public static class LengthExtensions
{
public static Length Millimeters(this double value)
{
return Length.FromMillimeters(value);
}

public static Length Millimeters(this decimal value)
{
    return Length.FromMillimeters((double)value);
}

// etc

}`

However, it might be presumptuous to assume that I know best, considering I've only just started with UnitsNet. I appreciate any guidance or suggestions you can provide on this matter.

Thank you for your attention and assistance.

[angularsen]

Nice find.

I see this is due to our quantities implementing IConvertible, added way back in #596 .

Why was IConvertible implemented?

I don't recall exactly, it may have been related to UI bindings in WPF and similar, or maybe it just seemed "useful" to support standard .NET interfaces.

You can then use Convert.ChangeType(Length.FromCentimeters(100), typeof(LengthUnit)) to get its unit enum LengthUnit.Centimeter, or its QuantityType enum, or its BaseDimensions.

These tests describe the intended use:
https://github.com/angularsen/UnitsNet/pull/596/files#diff-a1071ea2fda52ebdb297e17d5109d73e27f67c091c78dcd5157da7a4cb507441R125-R150

Why does it convert to double?

For better or worse, all numeric types are implemented, converting from Value property.
I'm not sure if there was a particularly good reason to add it, and we did make a note about it back then:

#596 (comment)

A bit unsure about converting to numbers like this, but hey, I guess it's the caller's funeral if they convert to number type and expected a different value :-)

How to fix it?

Option 1) Remove number conversions. Breaking change, could be added to #1200 . We don't know if anyone relies on this behavior or not.

Option 2) Restrict the generics to INumber in net7.0 and later.

🛡️Restrict number extension methods to INumber #1281

I think this would be my preferred option. What do you think?

cc @tmilnthorp

[angularsen]

Solved by #1281 in 5.25.0

[steelxjoel]

Thank you for validating my concerns and solving so fast. Love the 'caller's funeral' comment - I guess that could have been us :D Actually, we are still a little stranded in .net 4.6, so I won't be able to see the solution in action yet. Instead, I'm using vanilla UnitsNet and made int/double/decimal converters based on the source code in NumberExtensions. I don't think I'll share it because it is only done for the few types that we use, and you now have the INumber solution. I suppose if anyone asks for it, I could put it up as a fork that does not get merged.

[angularsen]

Makes perfect sense, glad you found workaround and then there is a solution waiting for you whenever, if ever, you migrate to net7.0 😄