0

I am trying to add a general-purpose TopN IEnumerable<T> extension.

If the parameter is positive then it is the same as Take() but if it is negative then it should do the same as Take() but then keep yielding immediately sequential values that match the last value from Take(). (The same as SQL TOP n WITH TIES)

This is the code I have at the moment:-

public static class Test
{
    public static IEnumerable<TSource> TopN<TSource>(this IEnumerable<TSource> source, int topN)
    {
        return TopN(source, topN, (v1, v2) => v1.Equals(v2));
    }

    public static IEnumerable<TSource> TopN<TSource>(this IEnumerable<TSource> source, int topN, Func<TSource, TSource, bool> comparer)
    {
        if (source == null) throw new ArgumentNullException(nameof(source));
        if (comparer == null) throw new ArgumentNullException(nameof(comparer));

        return topN >= 0
            ? source.Take(topN)
            : TopNWithTiesIterator(source, -topN, comparer);
    }

    static IEnumerable<TSource> TopNWithTiesIterator<TSource>(this IEnumerable<TSource> source, int topN, Func<TSource, TSource, bool> comparer)
    {
        var lastItem = default(TSource);

        foreach (var item in source)
        {
            if (topN-- > 0 || comparer(item, lastItem))
            {
                lastItem = item;

                yield return item;
            }
            else
            {
                yield break;
            }
        }
    }
}

and here is a sample of real-world usage and some other quick tests I tried:

        if (TopN != 0)
        {
            var values = new[] { 1, 2, 2, 3 };
            Debug.Assert(!values.TopN(0).Any());
            Debug.Assert(!values.TopN(0, (v1, v2) => v1 == v2).Any());

            Debug.Assert(values.TopN(1, (v1, v2) => v1 == v2).Count() == 1);
            Debug.Assert(values.TopN(-1, (v1, v2) => v1 == v2).Count() == 1);

            Debug.Assert(values.TopN(2, (v1, v2) => v1 == v2).Count() == 2);
            Debug.Assert(values.TopN(-2, (v1, v2) => v1 == v2).Count() == 3);

            Debug.Assert(values.TopN(2).Count() == 2);
            Debug.Assert(values.TopN(-2).Count() == 3);

            // This is how I really want to use it
            summaries = summaries.TopN(TopN, (v1, v2) => v1.ClientValue + v1.AdviserValue == v2.ClientValue + v2.AdviserValue);
        }

My question is about whether using Func<TSource, TSource, bool> as a comparer is correct.

Should I be using IEqualityComparer<T> or IEquatable<<T> or something else?

Simon Hewitt
  • 1,391
  • 9
  • 24
  • Trying to reword this question for it not to be opinion-based is hard. Would it helped if I had added "..to match existing Microsoft standards"? – Simon Hewitt Mar 19 '16 at 08:27

2 Answers2

1

Default and expected behavior should be:

  • If no IEqualityComparer<T> is provided, your method should check if TSource implements IEquatable<T> and use IEquatable<T>.Equals(T). Otherwise, it should use Object.Equals.

  • If an IEqualityComparer<T> is provided, then use it.

  • If a predicate is provided to mimic IEqualityComparer<T> behavior, then use it instead of an IEqualityComparer<T>.

Matías Fidemraizer
  • 63,804
  • 18
  • 124
  • 206
  • See comment below. I think that the anonymous type means the Func option will have to stay but I will also implement your suggestions to use IEquatable/Object.Equals rather than my simplistic Equals. Thanks to you both. – Simon Hewitt Mar 19 '16 at 08:34
  • @SimonHewitt Yeah, you can provide many overloads. But if you want to have a "complete" solution you should follow the criteria in my answer – Matías Fidemraizer Mar 19 '16 at 08:38
0

There's no choosing between Func<TSource, TSource, bool> and IEqualityComparer<T>. You're using default object's Equals comparer here (v1, v2) => v1.Equals(v2).

So it depends if you want flexibility to supply a custom comparer either via Func<> or directly as optional method argument.

vendettamit
  • 14,315
  • 2
  • 32
  • 54
  • Good point. I have come to the conclusion that I definitely need the Func option because the use I currently have is for an anonymous type so I can't implement IEquatable or IEqualityComparer on it. – Simon Hewitt Mar 19 '16 at 08:29