Another DataGrid (ObservableCollection) Filtering Method

Question

I have re-written my DataGrid filtering method to try to improve performance and I feel that although this is the fastest yet, there is still room for improvement on some of the slower machines my program runs on.

The DataGrid is bound to an ObservableCollection, as the user enters text into my searchBox each CompanyModel that matches the filter is added into a filter ListBox. The ListBox is limited to the first 25 CompanyModels so as to enhance performance. Here is the method that is called as the user types;

private void FilterCompanies()
{
    FilteredCompanies = new ObservableCollection<CompanyModel>();
    FilteredCompanies.Clear();

    string searchText = CleanseText(searchBox.Text.ToLower());

    foreach (CompanyModel company in AllCompanies)
    {
        if (!string.IsNullOrEmpty(searchText))
        {
            if (FilteredCompanies.Count <= 25)
            {
                if (CompanyMatchesFilters(company))
                {
                    if (CleanseText(company.Name.ToLower()).Contains(searchText) ||
                        CleanseText(company.Town.ToLower()).Contains(searchText) ||
                        CleanseText(company.Postcode.ToLower()).Contains(searchText))
                    {
                        FilteredCompanies.Add(company);
                    }
                }
            }
        }
    }

    if (FilteredCompanies.Count > 0)
    {
        companiesListBox.ItemsSource = FilteredCompanies;
        companiesListBox.Visibility = Visibility.Visible;
    }
    else
    {
        companiesListBox.ItemsSource = null;
        companiesListBox.Visibility = Visibility.Collapsed;
    }
}

From the code you can see there are two more methods involved in the filtering;

CompanyMatchesFilters

private bool CompanyMatchesFilters(CompanyModel company)
{
    foreach (FilterItem item in firstListBoxItems)
    {
        if (item.ID == 1 && company.CurrentStatus != 1)
        {
            return false;
        }
        if (item.ID == 2 && company.Subcontractor != 1)
        {
            return false;
        }
        if (item.ID == 3 && company.Supplier != 1)
        {
            return false;
        }
        if (item.ID == 4 && company.Planthire != 1)
        {
            return false;
        }
        if (item.ID == 5 && company.Architect != 1)
        {
            return false;
        }
        if (item.ID == 6 && company.QS != 1)
        {
            return false;
        }
        if (item.ID == 7 && company.ProjectManager != 1)
        {
            return false;
        }
        if (item.ID == 8 && company.StructEng != 1)
        {
            return false;
        }
        if (item.ID == 9 && company.ServiceEng != 1)
        {
            return false;
        }
    }
    return true;
}

CleanseText

private static Regex badChars = new Regex("[^A-Za-z0-9']");

private string CleanseText(string text)
{
    return badChars.Replace(text, "");
}

I would be very grateful if I could have some advice on;

• A: How to improve my code in general
• B(maybe more importantly): How to further improve the performance of my methods if at all possible

Answer 1

You can refactor this into LINQ query for (arguably) better readability.

AllCompanies.Where(c => CompanyMatchesFilters(c))
            .Where(c => CleanseText(company.Name).Contains(searchText)
                     || CleanseText(company.Town).Contains(searchText) 
                     || CleanseText(company.Postcode).Contains(searchText))
            .Take(25);

As for performance - nothing in the code you've shown strikes me as potential performance bottleneck. You will probably have to run a profiler to pinpoint it. One thing in particular you might want to look at is what triggers the search and how often it is executed. For example, if I enter 10 chars in search box, do I search 1 or 10 times? While the filtering logic looks fairly lightweight, the performance implications of recreating 25 item templates in UI on every keystroke can be severe (depending on how complex the DataTemplate is). Rx has some nice extensions that might help in throttling UI events.

P.S. Whats the point of removing all meaningful characters in CleanseText? What characters are you interested in then? This is extremely unclear, you should document that portion of your code.

Answer 2

Some changes I suggest:

    private void FilterCompanies()
    {
        FilteredCompanies = new ObservableCollection<CompanyModel>();
        //FilteredCompanies.Clear();  <- Pointless, you already cleared it when setting a new instance

        string searchText = CleanseText(searchBox.Text.ToLower());

        if (!string.IsNullOrEmpty(searchText)) //put outside foreach to avoid pointless foreach iteration
        {
            foreach (CompanyModel company in AllCompanies)
            {
                if (CompanyMatchesFilters(company))
                {
                    //.Contains() is not case sensitive, no point on .ToLower()
                    if (CleanseText(company.Name).Contains(searchText) || CleanseText(company).Contains(searchText) || CleanseText(company.Postcode).Contains(searchText))
                    {
                        FilteredCompanies.Add(company);
                        if (FilteredCompanies.Count <= 25)
                            break;
                    }
                }
            }
        }

        companiesListBox.ItemsSource = FilteredCompanies.Any() ? FilteredCompanies : null;
        companiesListBox.Visibility = FilteredCompanies.Any() ? Visibility.Visible : Visibility.Collapsed;
    }

Answer 3

CompanyMatchesFilters can be refactored as well. Just put all the company properties into an array. This will work because they seem to be of the same type.

private bool CompanyMatchesFilters(CompanyModel company)
{
    var values = new[]{
        company.CurrentStatus,
        company.Subcontractor,
        company.Supplier,
        company.Planthire,
        company.Architect,
        company.QS,
        company.ProjectManager,
        company.StructEng,
        company.ServiceEng
    }
    return !firstListBoxItems.Any(item => item.ID - 1 < values.Length 
        && values[item.ID - 1] != 1);
}

Answer 4

Putting all the optimizations and filtering logic aside... you're doing it wrong ;-)

---

With WPF we use the CollectionView which

Represents a view for grouping, sorting, filtering, and navigating a data collection.

e.g. to apply filters or sorting for the ListBox.

---

Let's say your ListBox is bound to a property of your view-model:

<ListBox ItemsSource="{Binding Companies}" />

where companies is an ICollectionView initilized like this:

public ICollectionView Companies { get; }

public CompaniesViewModel()
{
    var companies = GetCompanies();
    _companiesView = CollectionViewSource.GetDefaultView(companies);
}

---

Next step is to add a filter to it via the Filter property with the signature

public virtual Predicate<object> Filter { get; set; }

this means it

Gets or sets a method used to determine if an item is suitable for inclusion in the view.

---

So let's define the filter method that receives one company at a time that you can filter

private bool CompanyFilter(object item)
{
    var company = item as Company;

    // the optimized and pretty filtering logic ;-]

    return true/false...
}

and set the filter:

Companies.Filter = CompanyFilter;

---

Everything so far you do only once to prepare the filtering. What you need to repeat is changing the filter settings and refershing it by calling the Refresh method where and when suitable:

  Companies.Refresh();

---

There is more to this which you can read on wpftutorial - How to Navigate, Group, Sort and Filter Data in WPF

---

I'd like to add that setting the filtering method as presend applies to simple scenarios. In your case it would be better to encapsulate the entire filter logic and properties in a CompanyFilter class and use an instace of it in your view model. This way you can better test the filter and make sure it works as it should without involving other objects.

class CompanyFilter
{
    public bool FilterCompany(object item)
    {
        var company = item as company;

        // filtering...
    }

    // properties...

    // other filter helper methods.... but private
}

Answer 5

Way to many calls to CleanseText(company.Prop.ToLower())
And I would pull more search logic into Companay

public class Company
{
    private string name = string.Empty;
    private string cleanName = string.Empty;
    public string Name
    {
        get { return name; }
        set
        {
            if (name == value)
                return;
            name = value;
            NotifyPropertryChanged("Name");
            cleanName = CleanseText(name.ToLower());
            NotifyPropertryChanged("CleanName");
        }
    }
    public string CleanName
    {
        get { return cleanName; }
    }
    public bool MatchSearch(string searchText)
    {
        searchText = CleanseText(searchText);
        if (string.IsNullOrEmpty(searchText))
            return false;
        return (cleanName.Contains(searchText) ||
                cleanTown.Contains(searchText) ||
                cleanPostcode.Contains(searchText));
    }
}

I would even pull MatchesFilters into Company and pass in the collection of filters.

• Should not be binding companiesListBox.ItemsSource in FilterCompanies()

• Should avoid referencing UI elements in a method

• Should not be newing FilteredCompanies in FilterCompanies()

• Properly bind FilteredCompanies once - Clear and Add - that is what an ObservableCollection does