UnitsNet icon indicating copy to clipboard operation
UnitsNet copied to clipboard

Implicit cast between Duration and TimeSpan

Open Muximize opened this issue 1 year ago • 1 comments

See https://github.com/angularsen/UnitsNet/pull/1354#discussion_r1493344825

One issue is that the operator overloads only work when TimeSpan is the right operand.

I changed the code generation to take this into account, but another option would be to make a breaking change where we just don't support TimeSpan as the left operand at all.

Then users would have to cast explicitly, or for multiplication just reverse the operands.

This would affect 13 operators:

TimeSpan * Acceleration
TimeSpan * ElectricCurrent
TimeSpan * ElectricCurrentGradient
TimeSpan * ForceChangeRate
TimeSpan * KinematicViscosity
TimeSpan * MassFlow
TimeSpan * MolarFlow
TimeSpan * Power
TimeSpan * PressureChangeRate
TimeSpan * RotationalSpeed
TimeSpan * Speed
TimeSpan * TemperatureChangeRate
TimeSpan * VolumeFlow

Of which only 6 are used in tests so I assume are supported in v5:

TimeSpan * KinematicViscosity
TimeSpan * MassFlow
TimeSpan * Power
TimeSpan * RotationalSpeed
TimeSpan * TemperatureChangeRate
TimeSpan * Speed

Muximize avatar Feb 19 '24 17:02 Muximize

Hmm, these failing CI tests pass on my machine. Any guidance?

Muximize avatar Feb 19 '24 19:02 Muximize

@Muximize Beyond multiplication, are there any cases of TimeSpan as left hand argument?

I can't think of any, so I'd say let's just make a breaking change and not support it.

For multiplication, they can switch order. Otherwise, just do an explicit cast or maybe call an extension method .ToDuration() on it.

angularsen avatar Feb 23 '24 22:02 angularsen

As for the failing test case, that was my bad in a previous merged PR. Pushed a fix to this branch.

angularsen avatar Feb 23 '24 23:02 angularsen

I completely removed TimeSpan from Duration inference, which is a nice cleanup.

Also removed some tests for TimeSpan as the left operand. They all still have a counterpart that tests the reverse.

Muximize avatar Feb 24 '24 09:02 Muximize