views:

58

answers:

1

Hi,

I wrote a custom ordering LINQ extension method as below but I think it can be optimized for large results.

Here is the code :

    public static IEnumerable<T> OrderByAncesty<T>(this IEnumerable<T> source, Func<T, DateTime> dateSelector, Func<T, float> scoreSelector)
    {
        var original = source.ToList();
        var maxDate = source.Max(dateSelector);
        var list = from p in original
                   let date = dateSelector(p)
                   let score = scoreSelector(p)
                   let date1 = date.ToOADate()
                   let date2 = maxDate.ToOADate()
                   let ancesty = (1 - (float)date1 / (float)date2) * score
                   select new
                   {
                       TObject = p,
                       Ancesty = ancesty
                   };
        return list.OrderBy(p => p.Ancesty).Select(p => p.TObject);
    }
+3  A: 

Each "let" clause adds an extra level of delegation. You can improve things somewhat by removing them. You also don't need the anonymous type - or quite possibly the ToList() call. Additionally, there's no point in calling ToOADate() on maxDate every time.

public static IEnumerable<T> OrderByAncesty<T>(this IEnumerable<T> source, 
    Func<T, DateTime> dateSelector, Func<T, float> scoreSelector)
{
    var maxDate = (float) source.Max(dateSelector).ToOADate();
    return original.OrderBy(p =>  
              (1 - (float)dateSelector(p).ToOADate() / maxDate))
              * scoreSelector(p));
}

It's note as clear without the "let" clauses, mind you.

Jon Skeet
"Each "let" clause adds an extra level of delegation" : could you elaborate on this ? I thought it would create just one anonymous type with p, score, date1 and date2
Thomas Levesque
In order to make the "Descending" order, should I use OrderByAncesty(..).Reverse() ?
Yoann. B
Use OrderByDescending instead
John Gibb