6
\$\begingroup\$

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.

\$\endgroup\$
0

1 Answer 1

5
\$\begingroup\$

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.

\$\endgroup\$
3
  • \$\begingroup\$ The descAsc (descriptionAscending is the full term) part is used to check if the same column is clicked again, sorting the column in descending or ascending order. The default for each click in a column is ascending that is why I change the values to true to the other bool, then making the column's bool to false so that when it is clicked again, it orders it in descending order \$\endgroup\$ Aug 23, 2017 at 7:26
  • \$\begingroup\$ @Swellar oh, I see, I thought it meant descending-ascending... that's why you should not use abbreviations, especially unclear ones like this, that are known only to you ;-) \$\endgroup\$
    – t3chb0t
    Aug 23, 2017 at 7:27
  • \$\begingroup\$ That makes sense, I'll remember that. I'll make it descriptive even if it became longer. \$\endgroup\$ Aug 23, 2017 at 7:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.