Repository navigation
[API Proposal]: Expose the remaining const values in TimeSpan #94545
Description
Edited By @tarekgh
This proposal has been approved before #94545 (comment). There was some feedback from @tannergooding and @IDisposable.
@tannergooding said: the concern I raised is that there are scenarios this can cause problems since days * TimeSpan.HoursPerDay could overflow. For example, if days > 89,478,485 then this will overflow to a negative int.
HoursPerDay happens to be the only case where it is "sensible" for it to be int, since both MaxDays (10,675,199) and MaxHours (256,204,778) are within int.MaxValue. That is also why the signatures are FromDays(int days, int hours, long minutes, long milliseconds, long microseconds). So other values, like MinutesPerDay (1440), MinutesPerHour (60), or even the existing MillisecondsPerTick (10,000) are all expected to be used in contexts where long is the target type.
When considering TimeSpan, which is where these constants are defined, hours and days are int, all other units are long.
Background and motivation
In the TimeSpan class there are a few public const values that are generally applicable that are usefully exposed.
There are also a bunch more that are internal const but would be generally useful for other code and are not more implementation driven than the already exposed const values.
These values should be exposed publicly on the TimeSpan class.
Note: NOT suggesting we expose the truly internal const values that are related to MinTicks and MaxTicks. They could be included but are just not as naturally useful elsewhere.
API Proposal
namespace System.Collections.Generic;
public class TimeSpan: // elided details
{
// more elision
public const long MicrosecondsPerMillisecond = TicksPerMillisecond / TicksPerMicrosecond; // 1,000
public const long MicrosecondsPerSecond = TicksPerSecond / TicksPerMicrosecond; // 1,000,000
public const long MicrosecondsPerMinute = TicksPerMinute / TicksPerMicrosecond; // 60,000,000
public const long MicrosecondsPerHour = TicksPerHour / TicksPerMicrosecond; // 3,600,000,000
public const long MicrosecondsPerDay = TicksPerDay / TicksPerMicrosecond; // 86,400,000,000
public const long MillisecondsPerSecond = TicksPerSecond / TicksPerMillisecond; // 1,000
public const long MillisecondsPerMinute = TicksPerMinute / TicksPerMillisecond; // 60,000
public const long MillisecondsPerHour = TicksPerHour / TicksPerMillisecond; // 3,600,000
public const long MillisecondsPerDay = TicksPerDay / TicksPerMillisecond; // 86,400,000
public const long SecondsPerMinute = TicksPerMinute / TicksPerSecond; // 60
public const long SecondsPerHour = TicksPerHour / TicksPerSecond; // 3,600
public const long SecondsPerDay = TicksPerDay / TicksPerSecond; // 86,400
public const long MinutesPerHour = TicksPerHour / TicksPerMinute; // 60
public const long MinutesPerDay = TicksPerDay / TicksPerMinute; // 1,440
public const long HoursPerDay = TicksPerDay / TicksPerHour; // 24
}API Usage
if (TimeSpan.MinutesPerHour * numHours) > TimeSpan.MinutesPerDay * 7)
Console.WriteLine("That's really weeks");Alternative Designs
N/A
Risks
Since these are compile-time constant values, there would be no runtime cost in either memory or execution overhead. Additionally they would centralize basic real-world facts to one logical location (TimeSpan).
Activity
Expose the useful constant values in TimeSpan
IDisposable commentedon May 3, 2024
Is this something we can add to the API review?
tannergooding commentedon May 9, 2024
CC. @tarekgh, this is a relatively simple ask and I know I've had to redefine such constants myself in personal projects to help with various extension methods or other time related structs used (such as a
Timestampstruct used to track frame timings in a game engine).44 remaining items
bartonjs commentedon Jun 25, 2024
Video
Based on the domain (the
TimeSpantype itself), the belief is thatlongis correct for everything exceptHoursPerDay(int).While that's not so nice for
seconds * TimeSpan.MillisecondsPerSecondfor passing to (e.g.)new CancellationTokenSource(int), it is more appropriate for working with the TimeSpan type itself.IDisposable commentedon Jun 25, 2024
That makes perfect sense, let me know when/what the final should be and I will resubmit
tannergooding commentedon Jun 25, 2024
You should feel free to submit a PR matching it at any time now, we're in agreement as to the handling here now.
tarekgh commentedon Jun 25, 2024
@IDisposable
It is ready now so you can resurrect or create a new PR now.
IDisposable commentedon Jun 25, 2024
Working on it... do we have a decision about the casting at call-site or modifying the parameters type as mentioned here?
We had settled on casting at the call site, last I remember... and suspect we should go that route now.
Expose the constants in TimeSpan
IDisposable commentedon Jun 25, 2024
PR #103993 has hopefully final iteration :)