Skip to content
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

Proposal: Implementing IComparable should implement comparison operators #8633

Closed
iDaN5x opened this issue Feb 12, 2016 · 6 comments
Closed
Labels
Area-Language Design Feature Request Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.

Comments

@iDaN5x
Copy link

iDaN5x commented Feb 12, 2016

Implementing IComparable is sufficient for allowing comparison of two instances of class X.
It is, therefore, preferable that comparison operators (such as !=, =, <, <=, etc.) would be auto-implemented for X pairs when implementing IComparable.

Currently, large share of comparable classes features the following silly-looking pattern:

public static bool operator  < (Foo x, Foo y) { return CompareTo(x, y)  < 0; }
public static bool operator  > (Foo x, Foo y) { return CompareTo(x, y)  > 0; }
public static bool operator <= (Foo x, Foo y) { return CompareTo(x, y) <= 0; }
public static bool operator >= (Foo x, Foo y) { return CompareTo(x, y) >= 0; }
public static bool operator == (Foo x, Foo y) { return CompareTo(x, y) == 0; }
public static bool operator != (Foo x, Foo y) { return CompareTo(x, y) != 0; }

Moreover, by adding this feature, the following will be possible:

public void DoStuff<T>(T x, T y)  where T : IComparable {
    /* Not possible at the moment....
       Instead you're forced to use x.CompareTo(y) < 0
       Which is less readable */
    if (x < y) { 
        moreStuff();
    }
}

This is not such a drastic change but it'll be handy 😄

@svick
Copy link
Contributor

svick commented Feb 12, 2016

At the very least, automatically overloading == would be a breaking change. If a class currently implements IComparable<T>, but doesn't overload ==, then using == on it will use reference equality. But your proposal would change that to value equality.

@HaloFour
Copy link

Even with generics that would be a breaking change. The following is legal today and does perform a reference equality check:

public static bool IsEquals<T>(T x, T y) where T : class, IComparable<T>
{
    return x == y;
}

If the compiler would instead invoke IComparable<T>.Compare that could result in a different value or a NullReferenceException.

@iDaN5x
Copy link
Author

iDaN5x commented Feb 12, 2016

Was not considering this issue very deeply, and you're both definitely right.
This feature idea came to me while working, and I thought it could be nice to bring up.

Maybe it should be limited to <, <=, >, >=?
Not sure if this is worth the time and effort to implement at this form...
what do you think?

@svick @HaloFour

@alrz
Copy link
Member

alrz commented Feb 12, 2016

Probably #5165 or #258 and #5624 could reduce the verbosity of defining all these operators.

@AdamSpeight2008
Copy link
Contributor

< <= == != => > should not be automagically be implemented via implementing IComparable or IComparable<T>. If you want to have the "comparision operators" you can support this by use a lifted type that does. Exts.Comparisions.Classes.Operators

Also what those operators actually mean in the context of that object should be left for the programmer designing those objects.

@gafter
Copy link
Member

gafter commented Feb 15, 2016

The proposed change would break existing programs.

@gafter gafter closed this as completed Feb 15, 2016
@gafter gafter added Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Feature Request Area-Language Design labels Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Feature Request Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.
Projects
None yet
Development

No branches or pull requests

6 participants