HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Implementation of ApplySort method from IBindingList

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
applysortmethodibindinglistimplementationfrom

Problem

I am currently developing a family of keyed collections, and I wanted to have one which would implement the IBindingList interface.

My BindingKeyedCollection is derived from a KeyedCollection which owns a look-up dictionary, used to make the researches by key.

However, I have suspicions about my implementation of the ApplySort method (a method used to sort the collection relatively to one of the items property). It seems to be working as it is now, but I am not sure that all items are synchronized properly.

protected virtual void ApplySortCore(PropertyDescriptor property, ListSortDirection direction)
    {
        this.SortProperty = property;
        this.SortDirection = direction;
        if (direction == ListSortDirection.Ascending)
        {
            ResetItems(this.OrderBy(item => property.GetValue(item)).ToList());
            IsSorted = true;
            OnListChanged(new ListChangedEventArgs(ListChangedType.Reset, property));
        }
        else if (direction == ListSortDirection.Descending)
        {
            ResetItems(this.OrderByDescending(item => property.GetValue(item)).ToList());
            IsSorted = true;
            OnListChanged(new ListChangedEventArgs(ListChangedType.Reset, property));
        }
    }


Basically I am using LINQ to order another collection and change its Items list with the main one.

private void ResetItems(List items)
    {
        base.ClearItems();

        foreach (var item in items)
        {
            base.InsertItem(IndexOf(item), item);
        }
    }


I would like to have an external opinion on this way of sorting my collection, if they are any flaws that I would have missed, and if so, is there a way to avoid them?

Solution

The ApplySortCore() method could be made more dry

protected virtual void ApplySortCore(PropertyDescriptor property, ListSortDirection direction)
{
    this.SortProperty = property;
    this.SortDirection = direction;

    IEnumerable items = null;
    if (direction == ListSortDirection.Ascending)
    {
        items = this.OrderBy(item => property.GetValue(item));
    }
    else
    {
        items = this.OrderByDescending(item => property.GetValue(item));
    }

    ResetItems(items.ToList ());

    IsSorted = true;
    OnListChanged(new ListChangedEventArgs(ListChangedType.Reset, property));
}


You should always try to code against interfaces instead of a concrete implementation.

In the ResetItems() method you should either use an IList or better an IEnumerable for the input parameter.

The call to this IndexOf() method seems a little bit strange. You already have the items sorted why don't you just add the items one by one ?

Because the methods are inherited from Collection we can use the Add() method which simplifies the ResetItems() method to

private void ResetItems(IEnumerbale items)
{
    base.ClearItems();

    foreach (var item in items)
    {
        base.Add(item);
    }
}

Code Snippets

protected virtual void ApplySortCore(PropertyDescriptor property, ListSortDirection direction)
{
    this.SortProperty = property;
    this.SortDirection = direction;

    IEnumerable<T> items = null;
    if (direction == ListSortDirection.Ascending)
    {
        items = this.OrderBy(item => property.GetValue(item));
    }
    else
    {
        items = this.OrderByDescending(item => property.GetValue(item));
    }

    ResetItems(items.ToList ());

    IsSorted = true;
    OnListChanged(new ListChangedEventArgs(ListChangedType.Reset, property));
}
private void ResetItems(IEnumerbale<TItem> items)
{
    base.ClearItems();

    foreach (var item in items)
    {
        base.Add(item);
    }
}

Context

StackExchange Code Review Q#93708, answer score: 3

Revisions (0)

No revisions yet.