Repository navigation
Add generic overloads to Math.Min and Math.Max #14510
Description
There are overloads of Math.Min and Math.Max for all the numeric types, but there aren't any for general comparable types (including framework types like DateTime and TimeSpan). It would be nice to have them.
Rationale
For integral types, Math.Min and Math.Max are not necessary, e.g. instead of Math.Min(a, b), you can write a <= b ? a : b. But they are nice, because they make the meaning clear and decrease the likelihood of error. The same argument applies to general comparable types, so they should work with Math.Min and Math.Max too.
Proposed API
public static class Math
{
public static T Min<T>(T val1, T val2) where T : IComparable<T>;
public static T Min<T>(T val1, T val2, IComparer<T> comparer);
public static T Max<T>(T val1, T val2) where T : IComparable<T>;
public static T Max<T>(T val1, T val2, IComparer<T> comparer);
}Open questions
- Should the constraint
where T : IComparable<T>be there? Other framework methods that require comparison, likeEnumerable.OrderBy, don't have it and instead useComparer<T>.Default. - Should nullable value types be supported? The proposed version doesn't, because
T?does not implementIComparable<T?>, even whenTdoes implementIComparable<T>.
Activity
NickCraver commentedon May 14, 2016
I would ammend this API to move off of the
Mathclass, for 3 reasons:Mathceases to make as much sense with manyIComparable<T>types. You could be comparing swimming pools or classical music to see which is greater, these things aren't bounded to mathematical concepts.Instead, I'd suggest a new
staticclass calledCompare, where many such generic methods could live. Updated example:I would remove it and opt for a single overload that optionally accepts the
IComparer<T>. If it's null,Comparer<T>.Defaultwould be used for the same effect as the other overload.I don't think they should be supported - because defining this behavior is...hard to define. You could argue that
IComparable<T>forces such a comparison butstructs remain fun and ambiguous.benaadams commentedon May 14, 2016
No because nullable value types can and often represent database nulls and you don't want to go there... As it means closer to unknown and the comparisons and behaviours are almost philosophical.
Is a null greater or less than a value? Or is it indeterminate depending on what the unknown value really is?
Siderite commentedon May 17, 2016
I would use params to calculate the max/min for any number of items. And in that vein, if you are already using a list of objects, then a LInQ extension kind of makes more sense.
new[] { o1, o2, o3}.Max();NickCraver commentedon May 17, 2016
@Siderite that would require the an allocation of an array on each call. We definitely do not want that overhead. If such overhead is acceptable, then the current LINQ approach works.
Alternatively, we could provide such an overload, but alongside built-in non-array allocation overloads for (probably) 2, 3, and 4 parameters.
Siderite commentedon May 17, 2016
I always thought this was an antipattern resulting from the compiler laziness. Surely if there is a cost in allocating an array, then the compiler could create the underlying 1,2,3 parameter overloads for you. As such, with something like PostSharp, this could be implemented as a rule even by the developer, I think. Aren't you tired of writing
Math.Max(Math.Max(a,b),c)when you want to compare three numbers? Of course, one can always create a Linq aggregate, but it feels like overkill.NickCraver commentedon May 17, 2016
How would the compiler generate the code here? That's different code for the multiple cases - you'd spend far more time writing the necessary (and even if possible) compiler gymnastics here than the code for the few overloads :)
The whole point here (with any multi-overload method) is efficiency; optimizing for the 95-99% case with a few overloads and the rest can allocate more. If this wasn't a goal, we'd just all use the LINQ approach available right now.
danmoseley commentedon Dec 27, 2016
@AlexGhiondea what is next here, are we asking @svick to update his proposal? or is this ready for review?
AlexGhiondea commentedon Dec 28, 2016
@danmosemsft I believe we were still settling on the design. I will take a look when I get back next year!
AlexGhiondea commentedon Jan 4, 2017
@svick @NickCraver I am closing this since we are going to review basically the same in dotnet/corefx#11555