Finding the best way to sort my <T> list

Question

I am looking at a piece of code that I wrote not so long ago, and I just hate the way that sorting is handled - I wonder if anyone can show me the best way.

I have a Holding class that contains some information. I have another class, HoldingsList , which contains a List<Holding> member. I also have a PortfolioSheetMapping enumeration that has ~ 40 elements.

It looks something like this:

 public class Holding { public ProductInfo Product {get;set;} // ... various properties & methods ... } public class ProductInfo { // .. various properties, methods... } public class HoldingsList { public List<Holding> Holdings {get;set;} // ... more code ... } public enum PortfolioSheetMapping { Unmapped = 0, Symbol, Quantitiy, Price, // ... more elements ... } 

I have a method that can call a list that needs to be sorted, depending on which enumeration the user selects. The method uses the mondo switch statement, which has more than 40 cases (pah!).

The following is a short snippet of code:

 if (frm.SelectedSortColumn.IsBaseColumn) { switch (frm.SelectedSortColumn.BaseColumn) { case PortfolioSheetMapping.IssueId: if (frm.SortAscending) { // here I'm sorting the Holding instance's // Product.IssueId property values... // this is the pattern I'm using in the switch... pf.Holdings = pf.Holdings.OrderBy (c => c.Product.IssueId).ToList(); } else { pf.Holdings = pf.Holdings.OrderByDescending (c => c.Product.IssueId).ToList(); } break; case PortfolioSheetMapping.MarketId: if (frm.SortAscending) { pf.Holdings = pf.Holdings.OrderBy (c => c.Product.MarketId).ToList(); } else { pf.Holdings = pf.Holdings.OrderByDescending (c => c.Product.MarketId).ToList(); } break; case PortfolioSheetMapping.Symbol: if (frm.SortAscending) { pf.Holdings = pf.Holdings.OrderBy (c => c.Symbol).ToList(); } else { pf.Holdings = pf.Holdings.OrderByDescending (c => c.Symbol).ToList(); } break; // ... more code .... 

My problem is with the switch statement. switch is closely related to the PortfolioSheetMapping enumeration, which may change tomorrow or the next day. Each time it changes, I will have to revise this switch statement and add another case block to it. I'm just afraid that in the end this switch statement will be so large that it is completely uncontrollable.

Can someone tell me if there is a better way to sort my list?

Answer 1

You redirect the sorted data directly to the pf.Holdings property, so why not bypass the ToList and ToList and just use the Sort list instead?

You can use the map to place the Comparison<T> delegates for all supported sorts, and then call Sort(Comparison<T>) with the appropriate delegate:

 if (frm.SelectedSortColumn.IsBaseColumn) { Comparison<Holding> comparison; if (!_map.TryGetValue(frm.SelectedSortColumn.BaseColumn, out comparison)) throw new InvalidOperationException("Can't sort on BaseColumn"); if (frm.SortAscending) pf.Holdings.Sort(comparison); else pf.Holdings.Sort((x, y) => comparison(y, x)); } // ... private static readonly Dictionary<PortfolioSheetMapping, Comparison<Holding>> _map = new Dictionary<PortfolioSheetMapping, Comparison<Holding>> { { PortfolioSheetMapping.IssueId, GetComp(x => x.Product.IssueId) }, { PortfolioSheetMapping.MarketId, GetComp(x => x.Product.MarketId) }, { PortfolioSheetMapping.Symbol, GetComp(x => x.Symbol) }, // ... }; private static Comparison<Holding> GetComp<T>(Func<Holding, T> selector) { return (x, y) => Comparer<T>.Default.Compare(selector(x), selector(y)); } 

Answer 2

You can try to reduce the transition to the following:

  private static readonly Dictionary<PortfolioSheetMapping, Func<Holding, object>> sortingOperations = new Dictionary<PortfolioSheetMapping, Func<Holding, object>> { {PortfolioSheetMapping.Symbol, h => h.Symbol}, {PortfolioSheetMapping.Quantitiy, h => h.Quantitiy}, // more.... }; public static List<Holding> SortHoldings(this List<Holding> holdings, SortOrder sortOrder, PortfolioSheetMapping sortField) { if (sortOrder == SortOrder.Decreasing) { return holdings.OrderByDescending(sortingOperations[sortField]).ToList(); } else { return holdings.OrderBy(sortingOperations[sortField]).ToList(); } } 

You can fill out sortingOperations with reflection or save it manually. You can also make SortHoldings accept and return IEnumerable and delete ToList calls if you don't mind calling ToList on the caller later. I am not 100% sure that OrderBy is happy to receive the object, but it's worth taking a picture.

Edit: See LukeH's solution for everything to be strictly printed.

Answer 3

You viewed Dynamic LINQ

In particular, you can just do something like:

 var column = PortFolioSheetMapping.MarketId.ToString(); if (frm.SelectedSortColumn.IsBaseColumn) { if (frm.SortAscending) pf.Holdings = pf.Holdings.OrderBy(column).ToList(); else pf.Holdings = pf.Holdings.OrderByDescending(column).ToList(); } 

Note. This has the limitation that your listing matches your column names, if that suits you.

EDIT

I skipped the Product property for the first time. In these cases, DynamicLINQ will need to see, for example, "Product.ProductId" . You could reflect the name of the property, or simply use a "known" value and concat with the enumeration .ToString() . At the moment, I am simply forcing my answer to your question so that it is at least a working solution.

Answer 4

You can implement a custom IComparer class that uses reflection. However, it will be slower.

Here is the class I once used:

 class ListComparer : IComparer { private ComparerState State = ComparerState.Init; public string Field {get;set;} public int Compare(object x, object y) { object cx; object cy; if (State == ComparerState.Init) { if (x.GetType().GetProperty(pField) == null) State = ComparerState.Field; else State = ComparerState.Property; } if (State == ComparerState.Property) { cx = x.GetType().GetProperty(Field).GetValue(x,null); cy = y.GetType().GetProperty(Field).GetValue(y,null); } else { cx = x.GetType().GetField(Field).GetValue(x); cy = y.GetType().GetField(Field).GetValue(y); } if (cx == null) if (cy == null) return 0; else return -1; else if (cy == null) return 1; return ((IComparable) cx).CompareTo((IComparable) cy); } private enum ComparerState { Init, Field, Property } } 

Then use it as follows:

 var comparer = new ListComparer() { Field= frm.SelectedSortColumn.BaseColumn.ToString() }; if (frm.SortAscending) pf.Holding = pf.Holding.OrderBy(h=>h.Product, comparer).ToList(); else pf.Holding = pf.Holding.OrderByDescending(h=>h.Product, comparer).ToList(); 

Answer 5

What about:

 Func<Holding, object> sortBy; switch (frm.SelectedSortColumn.BaseColumn) { case PortfolioSheetMapping.IssueId: sortBy = c => c.Product.IssueId; break; case PortfolioSheetMapping.MarketId: sortBy = c => c.Product.MarketId; break; /// etc. } /// EDIT: can't use var here or it'll try to use IQueryable<> which doesn't Reverse() properly IEnumerable<Holding> sorted = pf.Holdings.OrderBy(sortBy); if (!frm.SortAscending) { sorted = sorted.Reverse(); } 

?

Not quite a quick solution, but it is quite elegant, what are you asking for!

EDIT: Oh, and with the case argument, it probably requires refactoring for a single function that Func returns, not a good way to completely get rid of it, but you can at least hide it from the middle of your procedure!

Answer 6

It seems to me that we can make two immediate improvements:

• the logic that uses frm.SortAscending to choose between OrderByDesccending and OrderByDesccending is duplicated in each case , and it can be pulled out after switch , if the case changed to do nothing but set the sort key and put it in Func

• which, of course, is left by the switch itself - and this can be replaced by a static map (in the Dictionary , say) from PortfolioSheetMapping to Func , which takes a Holding and returns the sort key. eg,

Answer 7

If the properties of the Hold class (symbol, price, etc.) are the same type, you can do the following:

 var holdingList = new List<Holding>() { new Holding() { Quantity = 2, Price = 5 }, new Holding() { Quantity = 7, Price = 2 }, new Holding() { Quantity = 1, Price = 3 } }; var lookup = new Dictionary<PortfolioSheetMapping, Func<Holding, int>>() { { PortfolioSheetMapping.Price, new Func<Holding, int>(x => x.Price) }, { PortfolioSheetMapping.Symbol, new Func<Holding, int>(x => x.Symbol) }, { PortfolioSheetMapping.Quantitiy, new Func<Holding, int>(x => x.Quantity) } }; Console.WriteLine("Original values:"); foreach (var sortedItem in holdingList) { Console.WriteLine("Quantity = {0}, price = {1}", sortedItem.Quantity, sortedItem.Price); } var item = PortfolioSheetMapping.Price; Func<Holding, int> action; if (lookup.TryGetValue(item, out action)) { Console.WriteLine("Values sorted by {0}:", item); foreach (var sortedItem in holdingList.OrderBy(action)) { Console.WriteLine("Quantity = {0}, price = {1}", sortedItem.Quantity, sortedItem.Price); } } 

which then displays:

Original values:
Quantity = 2, price = 5
Quantity = 7, price = 2
Quantity = 1, price = 3

Values sorted by Price:
Quantity = 7, price = 2
Quantity = 1, price = 3
Quantity = 2, price = 5