Removing the operators with `TimeSpan` in v6?

[lipchev]

I just found minor breaking change between v5 and v6 that I don't believe was mentioned anywhere:

        /// <summary>Get <see cref="Mass"/> from <see cref="TimeSpan"/> times <see cref="MassFlow"/>.</summary>
        public static Mass operator *(TimeSpan time, MassFlow massFlow)
        {
            return Mass.FromKilograms(massFlow.KilogramsPerSecond * time.TotalSeconds);
        }

        /// <summary>Get <see cref="Mass"/> from <see cref="MassFlow"/> times <see cref="Duration"/>.</summary>
        public static Mass operator *(MassFlow massFlow, Duration duration)
        {
            return Mass.FromKilograms(massFlow.KilogramsPerSecond * duration.Seconds);
        }

The TimeSpan overload has been removed (relying on the implicit conversion to Duration), and while we have the inverse overload in Duration (i.e. *(Duration duration, MassFlow massFlow)) the following expression is no longer correctly inferred:

var massEvaporated = (weightAssay.Date - currentValue.Date) * evaporationRate; // does not compile

Everything is fine if we inverted the order of operations:

var massEvaporated = evaporationRate * (weightAssay.Date - currentValue.Date); // no problem

I personally don't mind the removal (note that the implicit conversion from TimeSpan is lossless in my upcoming proposal for v6), but if we wanted - there would be no particular difficulty in having the operation overload added to the MassFlow.extra.cs.

I assume this affects all multiplications with TimeSpan (e.g. VolumeFlow and such)..

[angularsen]

I think it's important that UnitsNet don't add unnecessary friction on this, so if some operator overloads don't work with implicit cast to Duration, then we should add the corresponding TimeSpan overload.

[Muximize]

This was discussed here: https://github.com/angularsen/UnitsNet/pull/1354#discussion_r1493344825

And implemented here: https://github.com/angularsen/UnitsNet/pull/1365

[angularsen]

@Muximize I believe we can adapt #1365 to take into account left hand args of TimeSpan, and autogenerate explicit overloads for these cases, right?

[Muximize]

Yes, this will add 28 additional operators to the current set of units. I tested two ways of doing this:

1. Generating the operators exactly like the others, adding to QuantityRelationsParser.cs:

   var timeSpanQuantity = pseudoQuantity with { Name = "TimeSpan" };
   relations.AddRange(relations
   .Where(r => r.LeftQuantity.Name is "Duration")
   .Select(r => r with { LeftQuantity = timeSpanQuantity, LeftUnit = r.LeftUnit with { PluralName = "Total" + r.LeftUnit.PluralName } })
   .ToList());

2. Just reversing the operands so they use the implicit conversion, adding to QuantityGenerator.cs:

   if (relation.RightQuantity.Name is "Duration")
   {
   Writer.WL($@"
       /// <summary>Get <see cref=""{relation.ResultQuantity.Name}""/> from <see cref=""TimeSpan""/> {relation.Operator} <see cref=""{relation.LeftQuantity.Name}""/>.</summary>
       public static {relation.ResultQuantity.Name} operator {relation.Operator}(TimeSpan timeSpan, {relation.LeftQuantity.Name} {leftParameter}) => {leftParameter} {relation.Operator} timeSpan;
   ");
   }

[angularsen]

Without fully groking the full context here, I think solution 1 feels natural? I'm fine either way as long as there is a comment next to it explaining why.

[angularsen]

Nr2 seems the most straight forward though, but I was puzzled by relation.RightQuantity.Name, should it not be LeftQuantity? Was the operands swapped before this?

[Muximize]

Normally, if a quantity (say Power) has a relation with Duration, then power * duration will be in Power and duration * power will be in Duration

Because implicit conversion only works for the right operand, we have to generate timespan * power. We cannot add this to TimeSpan (not ours) and not to Duration (not part of the operator) so we have to add it to Power, so we infer it from power * duration. Hence the relation.RightQuantity.Name

However, as we do a similar thing with relations involving double, the first solution is a bit more natural indeed.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions[bot]]

This issue was automatically closed due to inactivity.