Problem comparing items implementing IComparable
Asked Answered
A

2

6

I am working on a extension method where it finds the min item by specific selector. Below the code

    public static T MinBy<T, K>(this IEnumerable<T> src, Func<T, K> selector) where K : struct, IComparable, IConvertible
    {
        var min = default(K);
        T minItem = default(T);
        foreach (var item in src)
        {
            var current = selector(item);
            if (current < min)
            {
                min = current;
                minItem = item;
            }
        }

        return minItem;

    }

It gives error Error Operator '<' cannot be applied to operands of type 'K' and 'K'. But i have specified the generic constraint K should be Struct and IComparable. I believe all the numeric data type can be satisfied with this.

Then how come this is a invalid operation.?

Assembled answered 24/2, 2011 at 6:55 Comment(0)
B
19

IComparable doesn't (and can't) say anything about operators. You should be using:

if (current.CompareTo(min) < 0)

Operators are static, and only ever overloaded rather than overridden. You can't require operators within interfaces, and the presence of a method doesn't magically change what an operator will do. (For example, overriding Equals does not change how == behaves.)

You should also note that as your constraint only talks about the nongeneric IComparable interface, you're going to be boxing at every operation. I would suggest you change the constraint to IComparable<K> instead. (Or drop the constraint and just use Comparer<K>.Default as Marc suggested.)

Some other comments about your method:

  • If all of the key values are more than the default value for K (e.g. K=int and all the keys are positive) then you won't find an item
  • You may wish to have an overload which accepts a particular IComparare<K> (but only if you drop the comparable constraint)
  • There's no real need to constrain K to a value type. What if I wanted to find the person with the lexicographically earliest name?
  • If there are no elements, it will return the default value for T; to fit in with the rest of LINQ I would suggest throwing InvalidOperationException
  • I would suggest using TSource and TKey as the type parameters to be more consistent with LINQ

You may want to look at the MoreLINQ MinBy implementation as an alternative. (Looking over that again, I'm not sure it's a good idea for us to require that comparer is non-null; it should probably use the default comparer in the same way as normal LINQ does if the comparer is null.)

Billups answered 24/2, 2011 at 6:57 Comment(1)
"For example, overriding Equals does change how == behaves" - that should probably say "does not change"?Rudin
I
11

IComparable doesn't provide operator support - you need to use current.CompareTo(min). Or better, use Comparer<T>.Default.Compare(current,min) - then you can drop the constraint and it'll handle nulls etc automatically, and it'll avoid boxing.

var comparer = Comparer<T>.Default;
...
// loop
    if(comparer.Compare(current, min) < 0) {...}
Ironlike answered 24/2, 2011 at 6:57 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.