-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[API Proposal]: method overload DateTimeOffset.Equals(DateTimeOffset other, TimeSpan margin)
#101393
Comments
Tagging subscribers to this area: @dotnet/area-system-datetime |
I wonder if exposing some |
...As a point of personal preference, date/time values should generally be considered equivalent to floating point (yes, it's generally exactly representable, but due to the measurement nature you can still end up with odd values). This tends to make me want to encourage one of two general behaviors:
|
I think this is already covered by today, isn't it? Like: var dt = DateTime.Parse("Wed, 21 Oct 2015 07:28:00 GMT");
bool isToday = DateTime.Today <= dt && dt < DateTime.Today.AddDays(1.0);
Is an "ulp" an error margin? Then the API would be something like: public readonly struct DateTimeOffset
{
public bool Equals(DateTimeOffset other, TimeSpan margin);
} I like this latter option too. |
Yes... but I'm hesitant to use a type that would exclude years or months, just on general principle. |
I think that using a |
The problem is that the proposed enum is generally too coarse. For instance, it doesn't allow for comparisons at a 10ms boundary... and that assumes that, especially at smaller measurements, truncation is the desired behavior. For instance, the historical "step" of consumer computer clocks was ~15ms, and there wasn't much control over when the value was for. Certain historical file times are limited to 2s precision. Things like that. |
What about providing both overloads @Clockwork-Muse ? The enum-based one for simpler, more common scenarios, and the |
The problem is that the "more advanced" one has different behavior. The proposed one truncates and buckets to the specified (very coarse) precision. Again, if the truncating/bucketing behavior is what you want, then from an application/domain design perspective probably you should be starting from the bucket (Eg, as if from an SQL calendar table) and stride the ranges. Note that in the majority of such cases, you're going to have the bounds anyways (you're not going to be comparing two arbitrary precision values). If you want "close to" values for arbitrary precision values, then you need an "error range", which means you need an absolute difference comparison, which this does not provide.
This is mostly a non-issue.
someValue.Equals(other, TimeDifference.Within10Milliseconds); |
@Clockwork-Muse , what do you think of the proposal below? @julealgon said we can have both overloads. namespace System;
public enum DateTimeComparison
{
UpToDate,
UpToHour,
UpToMinutes,
UpToSeconds,
UpToMilliseconds,
UpToMicroseconds,
UpToNanoseconds
}
public readonly struct DateTimeOffset
{
public bool Equals(DateTimeOffset other, DateTimeComparison comparisonType)
{
var thisTruncated = this.Truncate(comparisonType);
var otherTruncated = other.Truncate(comparisonType);
return thisTruncated == otherTruncated;
}
public bool Equals(DateTimeOffset other, TimeSpan margin)
{
var diffTicks = Math.Abs((this - other).Ticks);
var marginTicks = Math.Abs(margin.Ticks);
return diffTicks <= marginTicks;
}
} |
Potential benefits of a I think I would want to recommend some form of:
This is because a truncation comparison is essentially a "lossy" comparison (As opposed to the margin/ulp comparison) |
My opinion as a developer is that having a one-liner, straight-to-the-point method, is better for user experience, as I believe that in most cases this truncation would be used only for later comparison. We can add a |
@Clockwork-Muse , how about we go for both methods? public bool Equals(DateTimeOffset other, TimeSpan margin);
public bool Equals(DateTimeOffset other, DateTimeComparison comparisonType); The first one uses a margin. This is the equality check that xunit uses for assertion. I still believe that the second one is a good option, so the users won't need to declare a |
The fundamental problem is that truncation comparisons like the enum-based method would produce are almost always "wrong" from a domain design/algorithm/data perspective. In almost all real cases I can think of, the programmer should be starting from a bucket/striding perspective, as that will most closely match what it is they're actually trying to accomplish. Especially with the way the enum is designed. If you can provide me a reasonable counter example I'm willing to be proven wrong, however I imagine most of the cases you're thinking of a probably better served by the ulp comparison. |
I think I understand what you mean. var dt1 = DateTimeOffset.Parse("2024-07-09T17:34:22.800000-03:00");
var dt2 = DateTimeOffset.Parse("2024-07-09T17:34:23.100000-03:00");
bool truncateEquals = dt1.Equals(dt2, DateTimeComparison.UpToSeconds); // false
bool marginEquals = dt1.Equals(dt2, TimeSpan.FromSeconds(1)); // true
// we care for a 1s margin and there is a 300ms diff,
// but the two methods return different values. Considering that, I agree that the margin method is better. |
|
DateTimeOffset.Equals(DateTimeOffset other, DateTimeComparison comparisonType)
DateTimeOffset.Equals(DateTimeOffset other, TimeSpan margin)
Background and motivation
Currently, it is difficult in .NET to compare equality of DateTime(Offset) when parts need to be ignored, such as milliseconds and seconds. Check this Stack Overflow question.
API Proposal
Same for TimeSpan, DateTime and DateTimeOffset.
EqualsExact
would also have this overload, but considering TimeZoneOffset / DateTimeKind too.API Usage
Alternative Designs
Open for suggestions.
Risks
I don't see any, at first sight.
The text was updated successfully, but these errors were encountered: