Problem
I use these codes to re-order columns in a ListView
in ascending or descending order:
public static ObservableCollection<Inventory> SortedDailyCollection = new ObservableCollection<Inventory>();
public static ObservableCollection<Inventory> SortedCustomCollection = new ObservableCollection<Inventory>();
bool isDaily = true;
bool descAsc = true;
bool suppAsc = true;
bool soldAsc = true;
private void lvProductsColumnHeader_Click(object sender, RoutedEventArgs e)
{
GridViewColumnHeader column = (sender as GridViewColumnHeader);
string sortBy = column.Tag.ToString();
if (listViewSortCol != null)
{
AdornerLayer.GetAdornerLayer(listViewSortCol).Remove(listViewSortAdorner);
}
ListSortDirection newDir = ListSortDirection.Ascending;
if (listViewSortCol == column && listViewSortAdorner.Direction == newDir)
newDir = ListSortDirection.Descending;
listViewSortCol = column;
listViewSortAdorner = new SortAdorner(listViewSortCol, newDir);
AdornerLayer.GetAdornerLayer(listViewSortCol).Add(listViewSortAdorner);
switch (sortBy)
{
case "Description":
SortDescription();
break;
case "Supplier":
SortSupplier();
break;
case "Sold":
SortSold();
break;
default:
break;
}
}
private void SortDescription()
{
ObservableCollection<Inventory> chosenCollection = (isDaily) ? SortedDailyCollection : SortedCustomCollection;
List<Inventory> InventoryList = chosenCollection.ToList();
InventoryList = (descAsc) ? InventoryList = InventoryList.OrderBy(r => r.ProductDescription).ToList() : InventoryList = InventoryList.OrderByDescending(r => r.ProductDescription).ToList();
var tempCollection = new ObservableCollection<Inventory>(InventoryList);
chosenCollection.Clear();
foreach (var item in tempCollection)
{
Inventory _inventory = new Inventory
{
ProductDescription = item.ProductDescription,
Supplier = item.Supplier,
ProductSold = item.ProductSold,
ProductCost = item.ProductCost,
ProductRetail = item.ProductRetail,
TotalRetail = item.TotalRetail
};
chosenCollection.Add(_inventory);
}
listView.ItemsSource = chosenCollection;
if (descAsc)
{
descAsc = false;
suppAsc = true;
soldAsc = true;
}
else
{
descAsc = true;
}
}
private void SortSupplier()
{
ObservableCollection<Inventory> chosenCollection = (isDaily) ? SortedDailyCollection : SortedCustomCollection;
List<Inventory> InventoryList = chosenCollection.ToList();
InventoryList = (suppAsc) ? InventoryList = InventoryList.OrderBy(r => r.Supplier).ToList() : InventoryList = InventoryList.OrderByDescending(r => r.Supplier).ToList();
var tempCollection = new ObservableCollection<Inventory>(InventoryList);
chosenCollection.Clear();
foreach (var item in tempCollection)
{
Inventory _inventory = new Inventory
{
ProductDescription = item.ProductDescription,
Supplier = item.Supplier,
ProductSold = item.ProductSold,
ProductCost = item.ProductCost,
ProductRetail = item.ProductRetail,
TotalRetail = item.TotalRetail
};
chosenCollection.Add(_inventory);
}
listView.ItemsSource = chosenCollection;
if (suppAsc)
{
descAsc = true;
suppAsc = false;
soldAsc = true;
}
else
{
suppAsc = true;
}
}
private void SortSold()
{
ObservableCollection<Inventory> chosenCollection = (isDaily) ? SortedDailyCollection : SortedCustomCollection;
List<Inventory> InventoryList = chosenCollection.ToList();
InventoryList = (soldAsc) ? InventoryList = InventoryList.OrderByDescending(r => r.ProductSold).ToList() : InventoryList = InventoryList.OrderBy(r => r.ProductSold).ToList();
var tempCollection = new ObservableCollection<Inventory>(InventoryList);
chosenCollection.Clear();
foreach (var item in tempCollection)
{
Inventory _inventory = new Inventory
{
ProductDescription = item.ProductDescription,
Supplier = item.Supplier,
ProductSold = item.ProductSold,
ProductCost = item.ProductCost,
ProductRetail = item.ProductRetail,
TotalRetail = item.TotalRetail
};
chosenCollection.Add(_inventory);
}
listView.ItemsSource = chosenCollection;
if (soldAsc)
{
descAsc = true;
suppAsc = true;
soldAsc = false;
}
else
{
soldAsc = true;
}
}
Is there a way to reduce the booleans
to be used? There will be times that the columns will increase or decrease, thus the change in boolean
count. I went with this method so that I can check whether the same column is clicked again or not.
Note: The isDaily
changes based from date, but is not included here.
Solution
You do not need three methods for sorting because the only difference between them is the property that you use for sorting. You might as well have only one method that takes a Fuc<Inventory, TKey>
as a key selector and use it with OrderBy
and OrderByDescending
:
private void SortInventory<TKey>(Func<Inventory, TKey> keySelector)
{
ObservableCollection<Inventory> chosenCollection = (isDaily) ? SortedDailyCollection : SortedCustomCollection;
List<Inventory> InventoryList = chosenCollection.ToList();
InventoryList = (descAsc) ? InventoryList = InventoryList.OrderBy(keySelector).ToList() : InventoryList = InventoryList.OrderByDescending(keySelector).ToList();
.. unchanged
}
Then you can modify your switch
as
switch (sortBy)
{
case "Description":
SortInventory(x => x.ProductDescription);
break;
.. other cases
}
or create helper methods:
SortInventoryByDescription() => SortInventory(x => x.ProductDescription);
and use them in the switch
switch (sortBy)
{
case "Description":
SortInventoryByDescription();
break;
.. other cases
}
You should also be more specific when naming your methods. SortDescription
means you sort descriptions and not something else by description which is what you are actually doing.
ObservableCollection<Inventory> chosenCollection = (isDaily) ? SortedDailyCollection : SortedCustomCollection;
You are not using this ObservableCollection
at all so the variable can simply be var chosenCollection
or if you really want to use explicit types then IEnumerable<Inventory>
is all you need for sorting.
Now let’s get improve the descAsc
variable. This one is really confusing. Is it asc when it’s true
or false
? It’d be much easier to use if you named it isAscending
.
Then is the same line:
InventoryList = (descAsc) ? InventoryList = InventoryList.OrderBy(keySelector).ToList() : InventoryList = InventoryList.OrderByDescending(keySelector).ToList();
you don’t need so many assignments. It’s fine to do the ordering first and then call ToList
only once at the end and assign the result to InvenotryList
also only once:
InventoryList = (isAscending ? chosenCollection.OrderBy(keySelector) : chosenCollection.OrderByDescending(keySelector)).ToList();
Further below I don’t understand why you are recreating the Inventory
with the foreach
loop. Maybe it’s a requirement.
One of the most confusing parts is the one where you create the tempCollection
from InventoryList
and copy it to the chosenCollection
.
Why don’t you simply use LINQ to do this so it could be as simple as:
var chosenCollection = isDaily ? SortedDailyCollection : SortedCustomCollection;
var sortedInventory = isAscending ? chosenCollection.OrderBy(keySelector) : chosenCollection.OrderByDescending(keySelector);
var sortedInventoryCopies =
from item in sortedInventory
select new Inventory
{
ProductDescription = item.ProductDescription,
Supplier = item.Supplier,
ProductSold = item.ProductSold,
ProductCost = item.ProductCost,
ProductRetail = item.ProductRetail,
TotalRetail = item.TotalRetail
};
listView.ItemsSource = new ObservableCollection<Inventory>(sortedInventoryCopies);
You don’t need any lists or multiple observable-collections. Just sort the inventory and pass the result to the final observable-collection which will become your ItemsSource
.
if (descAsc)
{
descAsc = false;
suppAsc = true;
soldAsc = true;
}
else
{
descAsc = true;
}
I cannot reason about this part. It’s too confusing. You should rethink it and also change the names to full words instead of abbreviations.
Another issue is that your code isn’t consistant. Once you use full type names and another time just var
. Why not use var
everywhere? Then you mix PascalCasing InventoryList
with camelCasing chosenCollection
for your variables. You should also use variable names that better describe things they hold. SortedDailyCollection
or SortedCustomCollection
don’t say anything about the content of the collection. SortedDailyInventory
or SortedCustomInventory
sound much better.