views:

116

answers:

3

I'm not quite sure I'm understanding how I can utilize generics in C# properly. Say I have the following method. I would like to allow it to work on Lists of any type. Currently I have List where Row is a custom struct, I want to reuse this sort method for half a dozen structs that I make. I thought I could just do List<T> in the return type and parameter type but it doesn't like that.

public static List<Row> SortResults( List<Row> listRow, string sortColumn, bool ascending)
        {
            switch (ascending)
            {
                case true:
                    return (from r in listRow
                            orderby r.GetType().GetField(sortColumn).GetValue(r)
                            select r).ToList<Row>();
                case false:
                    return (from r in listRow
                            orderby r.GetType().GetField(sortColumn).GetValue(r) descending
                            select r).ToList<Row>();
                default:
                    return listRow;
            }
        }
+8  A: 

Well, here's your original code made generic:

public static List<T> SortResults<T>(List<T> listRow, 
                                     string sortColumn, bool ascending)
{
    switch (ascending)
    {
        case true:
            return (from r in listRow
                    orderby r.GetType().GetField(sortColumn).GetValue(r)
                    select r).ToList();
        case false:
            return (from r in listRow
                    orderby r.GetType().GetField(sortColumn).GetValue(r) descending
                    select r).ToList();
        default:
            return listRow;
    }
}

Note how I've removed the type argument from the ToList calls - let the compiler work it out :)

(As an aside, I don't know why the compiler requires a default case here. Maybe it just always assumes there will be unlisted potential values.)

However, this can be made nicer:

public static List<T> SortResults<T>(List<T> listRow, 
                                     string sortColumn,
                                     bool ascending)
{
    FieldInfo field = typeof(T).GetField(sortColumn);
    Func<T, object> projection = t => field.GetValue(t);
    IEnumerable<T> sequence = ascending ? listRow.OrderBy(projection)
        : listRow.OrderByDescending(projection);
    return sequence.ToList();
}

This is still fairly inefficient (as it's using reflection) but at least it's not getting the FieldInfo separately for each item. If you want better performance, you can have a generic helper type which caches a delegate mapping a value to the field's value for each field, and then fetch that delegate once at the start of the method. It would definitely be more work, but I'd expect it to be an order of magnitude faster.

Jon Skeet
Jon, I remember being confused by the default requirement for a boolean switch as well. Eric Lippert had a great writeup that describes why (http://blogs.msdn.com/ericlippert/archive/2009/08/13/four-switch-oddities.aspx) - case four is the one of interest. His punchline is that the "reachability analyzer is not very smart", but it doesn't really matter.
jball
@Jon Skeet: Perhaps I'm doing something wrong, but I am only seeing the cached delegate version to be around twice to three times as fast.
Jason
@Jason: That does surprise me... but I'd have to see the code to know whether there's some reason, or whether I just had expectations which were too high...
Jon Skeet
@Jon Skeet: I put my code on pastebin (http://pastebin.com/f77e71397). On my machine the non-cached version takes around 105 seconds and the cached version takes around 47 seconds when ran as coded. Criticize away :-)
Jason
@Jason: That code isn't really thread-safe - I would initialize it with *all* the accessible fields in the static initializer. However, that wouldn't account for the speed difference. May try to play around with it myself some time, but I expect I just thought reflection was slower than it really is.
Jon Skeet
+1  A: 

You need to make the method itself generic:

public static List<Row> SortResults<Row>( List<Row> listRow, ...
                                   ^^^^^
itowlson
+2  A: 
public static List<T> SortResults( List<T> listObject, string sortColumn, bool ascending) where T: class

Then your ToList would be ToList<T>();

T represents your entity/object type.

Randy Minder
This is missing the type parameter on the method, so won't compile. You need to change `SortResults(` to `SortResults<T>(`.
Phil Ross
Yes, good catch.
Randy Minder