-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
TimeSpan has all of the semantics of numbers, but none of the new interfaces #76225
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
IMO all types having operator overloads should implement their corresponding operator interfaces. If a type is not number, it only isn't necessary to implement the INumberBase/INumber interface. |
Tagging subscribers to this area: @dotnet/area-system-numerics Issue DetailsBackground and motivationTimeSpan is fundamentally a long, and has most of the semantics of being a long. API Proposalnamespace System;
public class TimeSpan
: IComparable,
IComparable<TimeSpan>,
IEquatable<TimeSpan>,
ISpanFormattable,
ISpanParsable<TimeSpan>,
//NEW
IMinMaxValue<TimeSpan>,
IAdditionOperators<TimeSpan, TimeSpan, TimeSpan>,
IAdditiveIdentity<TimeSpan, TimeSpan>,
IEqualityOperators<TimeSpan, TimeSpan, bool>,
ISubtractionOperators<TimeSpan, TimeSpan, TimeSpan>,
IUnaryPlusOperators<TimeSpan, TimeSpan>,
IUnaryNegationOperators<TimeSpan, TimeSpan>,
ISignedNumber<TimeSpan> //<-- problem with this, see below
{
...
} API Usagevar min = Math.Min(TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(1)); Alternative DesignsNo response RisksA few semantics of the new interfaces does not really make sense for TimeSpan, e.g. TimeSpan can be negative, but the concept of NegativeOne does not really make much sense outside being a unary operator. and for some reason
|
The interfaces was implemented in the first version, but then removed when they were moved into |
As indicated above, the original design had types like
The interface name implies that it is a |
For #69590 would be nice to have |
For comparison, |
I think Ideally it should implement something more generic like Then ideally Regarding |
Concepts like Such concepts exist in a purely mathematical sense where you have a theoretical "turing machine" with infinite time/resources. However, in the practical sense no such computer exists. Applications generally run on an imperative machine where such machine has finite resources and finite time. Data is broken up into smaller pieces for easy interaction. Interacting with data can often have side effects and it can imply introduced error in the form of rounding, wrapping, saturating, truncating, or more. Such systems are typically minimally abstracted over with a basic type system. While the complexity of some type systems can vary, they all have limitations that further restrict what models can be exposed and how easy vs complex it is for others to consume the expose interface. In the case of Because of this, among other reasons, we have no plans to add such interfaces now or in the future. It is not the goal or aim of the BCL to try and provide mathematical abstractions like this. The interfaces we expose take into account the practical limitations and interaction scenarios a developer requires. We also take into account that things like
Such a concept would make sense for |
Well, the solution is easy, let's pick a different name, and don't enforce strict adherence to those properties.
I'm totally fine with that, as that's not the goal. |
You can just use
There is no reason why these interfaces can't be used elsewhere. They ultimately are very numeric oriented and are hidden away here because direct usage isn't as common. It's primarily a consideration for library/type authors rather than for application authors. API review already discussed and considered this when deciding to put these interfaces in
You're going to need to provide some more concrete examples of what you think is missing and usage scenarios around how you expect such an abstraction to be used/be beneficial. In the case of string/list/array, most of the functionality is covered by interfaces in |
Maybe also add would improve usability in constrained generic methods. |
Tagging subscribers to this area: @dotnet/area-system-datetime Issue DetailsBackground and motivationTimeSpan is fundamentally a long, and has most of the semantics of being a long. API Proposalnamespace System;
public class TimeSpan
: IComparable,
IComparable<TimeSpan>,
IEquatable<TimeSpan>,
ISpanFormattable,
ISpanParsable<TimeSpan>,
//NEW
IMinMaxValue<TimeSpan>,
IAdditionOperators<TimeSpan, TimeSpan, TimeSpan>,
IAdditiveIdentity<TimeSpan, TimeSpan>,
IEqualityOperators<TimeSpan, TimeSpan, bool>,
ISubtractionOperators<TimeSpan, TimeSpan, TimeSpan>,
IUnaryPlusOperators<TimeSpan, TimeSpan>,
IUnaryNegationOperators<TimeSpan, TimeSpan>,
ISignedNumber<TimeSpan> //<-- problem with this, see below
{
...
} API Usagevar min = Math.Min(TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(1)); Alternative DesignsNo response RisksA few semantics of the new interfaces does not really make sense for TimeSpan, e.g. TimeSpan can be negative, but the concept of NegativeOne does not really make much sense outside being a unary operator. and for some reason
|
FYI NodaTime is adding generic math interfaces to their types (nodatime/nodatime#1693). |
Background and motivation
TimeSpan is fundamentally a long, and has most of the semantics of being a long.
Yet, when adding the
IMinMaxValue<T>
,INumberBase<T>
and so on, none of them were applied to TimeSpan.This prevents making things like Math.Min (if it had an overload that used the new interfaces) work on TimeSpans.
API Proposal
API Usage
Alternative Designs
No response
Risks
A few semantics of the new interfaces does not really make sense for TimeSpan, e.g. TimeSpan can be negative, but the concept of NegativeOne does not really make much sense outside being a unary operator. and for some reason
ISignedNumber<T>
assumes the entireINumberBase<T>
is applicable.The text was updated successfully, but these errors were encountered: