-
Notifications
You must be signed in to change notification settings - Fork 21
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
Satisfy the "comparison" constraint with IComparable<'T> #816
Comments
I haven't seen a similar suggestion, though I'd be surprised if this hasn't been brought up before... ...regardless, I think this would be great. It's very surprising to me that the non-generic one satisfies, but the generic one does not. Also, I really wish equality and comparison in .NET wasn't so difficult. Just a quick glance at .NET FX for comparison
So if we're going by implementations/references, then the non-generic |
@cartermp, keep in mind that many of them are either in private or internal types, or in the likes of And you were talking about .NET Framework. I guess this has changed with .NET Core. |
I would expect none of that to change with .NET Core, at least for any existing API, otherwise it would be an API breaking change. |
I’m referring to more public types implementing the generic interfaces.
|
While I'm in favor of this, and I vaguely remember this has been discussed before, it may not be so trivial as it seems.
I'm not sure this is true. Considering that the compiler currently expects the non-generic At best I see this moving forward if the compiler can take advantage of the fact that the generic interface is implemented (not sure if it currently does). But simply allowing the In addition, when you implement (that is not to say that it would be good to have this, and esp with value types it can yield a far better performance too) |
@abelbraaksma Currently, these are the rules:
(section 5.2.10 of the spec) This suggestion would change it to be (bolded difference):
I don't believe that would be a breaking change. Existing nongeneric code that doesn't meet the current rules would continue to fail to satisfy that constraint. |
This advice is for C# developers.
If you would only implement Then the compiler would still implement |
@cartermp, You are right, maybe my concern was less of a concern than I thought. However, I still think it is a breaking change, though arguably much smaller, and probably only in edge cases:
The problem is with "or". Previously, any code that expects a type having the I believe that when the BCL was faced with the same issue, they typically introduced different methods for sorting: either generic, or non-generic. Perhaps this all can be inferred and none of my concerns above will end up becoming real-world problems, but on the face of it, I think there are certainly risks involved. (again, still would love this in, if it is possible and with little backwards-compatibility issues) |
Just some thoughts/ideas (not necessarily good ones):
|
how would this interact with the language suggestion around allowing multiple implementations of interfaces with distinct generic arguments? Would the derived IComparable implementations type-test and invoke the an appropriate matching member? If so, what order would the types be checked? Declaration order? Some normalized name ordering? |
I think this goes together with IEquatable and IEquatable. Can those be added to the suggestion? There was work done on IEquatable in dotnet/fsharp#6175 and I hope that can be returned to.
Even though that is still the official advice (https://docs.microsoft.com/en-us/dotnet/api/system.iequatable-1?view=net-5.0), the non-generic interfaces are only needed to support non-generic collections which are obsolete . So I think the advice is unnecessary at this point. It is so horrible to have to write: type T(value:int) =
member t.Value = value
interface System.IEquatable<T> with member x.Equals y = x.Value = y.Value
override x.Equals(yObj:obj) =
match yObj with
| :? T as y -> x.Value = y.Value
| _ -> failwith "incompatible types"
override x.GetHashCode() = 0 |
One aspect that isn't mentioned is also C# interop. I've just been working with a class defined in a .NET library over which I have no control. The class implements a custom IComparable <> and I was unable to use seq.Max as a result. I've had to create a wrapper type implementing IComparable as a workaround. As a user I found it irritating that such a fundamental interface is not supported, especially since I was left scratching my head for quite a while before understanding what was happening. This is not the sort of limitation I would have expected. |
I propose we allow a class that implements just the generic
IComparable<'T>
interface, satisfy thecomparison
constraint.The existing way of approaching this problem in F# is by implementing the non-generic interface
IComparable
(orIStructuralComparable
). However this approach is not ideal, because it requires a type cast (and potentially unboxing) of a value of typeobj
. Implementing all three interfaces would increase boilerplate code even more.Pros and Cons
The advantages of making this adjustment to F# are increased ease of defining a custom comparison function, and conformity to modern coding practices by using generic interfaces.
The disadvantages of making this adjustment to F# are none, I guess. As for backwards compatibility, we can provide an automatic implementation of the two non-generic interfaces, if there are not specified.
Extra information
Estimated cost (XS, S, M, L, XL, XXL): S
Related suggestions: The issue was first reported in dotnet/fsharp#7945.
Affidavit (please submit!)
Please tick this by placing a cross in the box:
Please tick all that apply:
The text was updated successfully, but these errors were encountered: