MadsKirkFoged / EngineeringUnits

Working with units made easy with automatic unit-check and converting between units
MIT License
42 stars 10 forks source link

Adding rounding extensions #58

Closed mschmitz2 closed 3 months ago

mschmitz2 commented 3 months ago

@MadsKirkFoged let me know what you think about the implementation and naming of this. It's just something I needed a few times in a project so I though I'd add it to the library.

I chose to implement this using generic methods with T : BaseUnit, e.g.

public static UnknownUnit? RoundTo<T>(this T? value, T? roundVal) where T : BaseUnit

... so a user is forced to use the same Unit for value and roundValue, i.e. you can't round a Volume to a Mass etc. Alternatively, one could implement it like

public static UnknownUnit? RoundTo(this BaseUnit? value, BaseUnit? roundVal)

and rely on the runtime checks in Ratio to throw an exception.

MadsKirkFoged commented 3 months ago

I think we should try it out. It could be nice if you didn't need to cast it back to the unit is was.

But I don't know if something like this would be able to compile?

public static T? RoundTo(this T? value, T? roundVal)

mschmitz2 commented 3 months ago

Yes this would be nice and I had something similar in mind at some point as well and played around a bit. I think there is no easy way to do this the way BaseUnit and the units inheriting from in is implemented right now.

This kind of shows you the issue:

image

... you cannot cast to T, and there is no constructor so you can't use new T().

I think this might work if BaseUnit were an abstract class that defined abstract constructors etc. But this would required quite a lot of changes as the current code uses BaseUnit quite a lot as a non-abstract class (I tried at some point and eventually decided is was not worth it). Maybe it's easy enough to create an abstract base class that BaseUnit inherits from. Not sure and I didn't try that.

mschmitz2 commented 3 months ago

The easier way to solve this would I think be to add such methods to the respective unit classes (not as extensions), using codegen.

You'd end up having the below, which compiles just fine:

image

This would do away with casting on the user side, which is nice, but it might not improve performance since you still do double * BaseUnit, which returns UnknownUnit, that is then cast to the result type (which might be fixable, see below).

Something like this might be useful for other methods as well. E.g. replacing the current AbsExtensions by non-static methods in the classes:

image

image

Similarly, there might be performance gained from creating overrides of quite a lot of methods defined in BaseUnit. For example plus, minus, equality operators etc where left and right are the same unit so we know the result is that unit, or multiplication by a double, which doesn't change the unit. Operations where left and right are different units should then still default to the methods in BaseUnit as currently.

This is not something I wanted to mess with for now but we can put it out there.