2
\$\begingroup\$

I'm having a lot of trouble in my Database program with trying to implement an effective DataGrid filtering method. After taking advice from a previous code review I'm trying to implement a DataModel method of coding. My previous method of searching my DataGrid was this;

SearchGrid (PREVIOUS)

private void SearchGrid(object sender, TextChangedEventArgs e)
    {
        DataView dv = dataGrid.ItemsSource as DataView;
        if (compNameRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyName LIKE '%" + searchBox.Text + "%'";
        }
        if (compTownRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyTown LIKE '%" + searchBox.Text + "%'";
        }
        if (compPcodeRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyPcode LIKE '%" + searchBox.Text + "%'";
        }
    }

This worked fine with the way I bound my DataGrid before. I now bind my DataGrid using a Model class and the previous filter method does not work. I have written a new filter method which is this;

SearchGrid (NEW)

private void SearchGrid(object sender, TextChangedEventArgs e)
        {
            if (!string.IsNullOrEmpty(searchBox.Text))
            {
                ICollectionView view =      CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
                view.Filter += (obj) =>
                {
                    CompanyModel model = obj as CompanyModel;
                    if (model == null)
                        return true;

                    if (compNameRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
                    }
                    if (compTownRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
                     }
                    if (compPcodeRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
                    }
                    return false;
                };
            }
        }

Now this does work OK, however there are two issues:

  1. is that it is extremely slow when I type text to search the DataGrid, and
  2. it doesn't deal with the most simple of cases, for example if the name of the Company is "A1" then searching for "a1" will not find the company.

I have been recommended to use Entity Framework however as I am using an OleDB connection (this is a very old Database I am rewriting) I have been unable to get this to work.

Is there a more efficient search method I can use that is quicker and would also find all companies (i.e case insensitive)?

\$\endgroup\$
5
\$\begingroup\$
private void SearchGrid(object sender, TextChangedEventArgs e)
    {
        if (!string.IsNullOrEmpty(searchBox.Text))
        {
            ICollectionView view =      CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
            view.Filter += (obj) =>
            {
                CompanyModel model = obj as CompanyModel;
                if (model == null)
                    return true;

                if (compNameRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
                }
                if (compTownRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
                }
                if (compPcodeRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
                }
                return false;
            };
        }
    }  

The problem with your code is that each time the text changed you are adding a new Filter to the ICollectionView by using

view.Filter += (obj) =>  

this little + is doing all the harm. For each raise of the TextChanged event a new filter is added and never removed.

If you remove the + you will see that your filtering will become faster.


The problem with A1 vs a1 is that the Contains() method is case sensitive. A better way would be to use IndexOf(string, StringComparison) like so

return !string.IsNullOrEmpty(model.CompanyName)  
       && model.CompanyName.IndexOf(searchBox.Text, StringComparison.OrdinalIgnoreCase) > -1;
\$\endgroup\$
  • \$\begingroup\$ Well, have to say that I totally ignored the + behavior on that bit. Guess you always learn not to underestimate the "small" bits :) \$\endgroup\$ – Gentian Kasa Oct 19 '15 at 9:59
  • \$\begingroup\$ I appreciate your answer @GentianKasa and will work on your feedback though I have accepted this answer for the increase in filtering speed :) \$\endgroup\$ – CBR Oct 19 '15 at 10:06
  • \$\begingroup\$ @CBR, I totally agree with your choice :) \$\endgroup\$ – Gentian Kasa Oct 19 '15 at 10:09
3
\$\begingroup\$

Some small tweaks could be:

  • Access the value of searchBox.Text only once and memorize it in a string that you use in the Filter event handler.
  • use stringValue != null instead of !string.IsNullOrEmpty(stringValue). In this way you avoid a method call and a check (if stringValue is empty) that is implicitly done in the Contains(string) method (string.Empty.Contains(stringValue) returns false for any stringValue != string.Empty)
  • If possible, make it so the Filter delegate takes a CompanyModel instead of object.

The final result should be something like the following (not tested):

private void SearchGrid(object sender, TextChangedEventArgs e)
{
    if (string.IsNullOrEmpty(searchBox.Text))
    {
        return;
    }

    string searchText = searchBox.Text;
    ICollectionView view = CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
    view.Filter += (model => 
    {
        if (model == null)
        {
            return true;
        }
        else if (compNameRad.IsChecked)
        {
            return model.CompanyName != null && model.CompanyName.Contains(searchText);
        }
        else if (compTownRad.IsChecked)
        {
            return model.CompanyTown != null && model.CompanyTown.Contains(searchText);
        }
        else if (compPcodeRad.IsChecked)
        {
            return model.CompanyPcode != null && model.CompanyPcode.Contains(searchText);
        }
        return false;
    };
}

and, by taking a look at the delegate, you could transform it (but it becomes a little less readable/understandable) into something like:

private void SearchGrid(object sender, TextChangedEventArgs e)
{
    if (string.IsNullOrEmpty(searchBox.Text))
    {
        return;
    }

    string searchText = searchBox.Text;
    ICollectionView view = CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
    view.Filter += (model => 
    {
        return (model == null)
            || (compNameRad.IsChecked && model.CompanyName != null && model.CompanyName.Contains(searchText))
            || (compTownRad.IsChecked && model.CompanyTown != null && model.CompanyTown.Contains(searchText))
            || (compPcodeRad.IsChecked && model.CompanyPcode != null && model.CompanyPcode.Contains(searchText));
    };
}

which behaves in the same way as the original method by using only the lazy evaluation feature of the || and && operator.

Regarding the search problem related to the case of the strings I'd just apply a ToLower() or ToUpper() to both strings. The end result should become the following:

private void SearchGrid(object sender, TextChangedEventArgs e)
{
    if (string.IsNullOrEmpty(searchBox.Text))
    {
        return;
    }

    string searchText = searchBox.Text.ToLower();
    ICollectionView view = CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
    view.Filter += (model => 
    {
        return (model == null)
            || (compNameRad.IsChecked && model.CompanyName != null && model.CompanyName.ToLower().Contains(searchText))
            || (compTownRad.IsChecked && model.CompanyTown != null && model.CompanyTown.ToLower().Contains(searchText))
            || (compPcodeRad.IsChecked && model.CompanyPcode != null && model.CompanyPcode.ToLower().Contains(searchText));
    };
}

Let me know if anything is unclear.

\$\endgroup\$
2
\$\begingroup\$

First, doing variable == true is redundant; variable by itself tests the true value.

Second, these conditions don't work in conjunction, so make them if else statements instead:

private void SearchGrid(object sender, TextChangedEventArgs e)
        {
            if (!string.IsNullOrEmpty(searchBox.Text))
            {
                ICollectionView view =      CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
                view.Filter += (obj) =>
                {
                    CompanyModel model = obj as CompanyModel;
                    if (model == null){
                        return true;
                    }

                    if (compNameRad.IsChecked) {
                        return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
                    } else if (compTownRad.IsChecked) {
                        return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
                    } else if (compPcodeRad.IsChecked) {
                        return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
                    }
                    return false;
                };
            }
        }
  • The indentation on your opening brackets is incorrect.

  • I would swap the string.IsNullOrEmpty check at the beginning around to remove the extraneous layer.

  • The spacing on the ICollectionView view is incorrect.

    • When using Linq, shouldn't (obj) => {obj.isLive}; be: (obj => {obj.isLive});

With those changes in mind:

private void SearchGrid(object sender, TextChangedEventArgs e)
{
    if (string.IsNullOrEmpty(searchBox.Text))
    {
        return;
    }
    ICollectionView view = CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
    view.Filter += (obj => 
    {
        CompanyModel model = obj as CompanyModel;
        if (model == null)
        {
            return true;
        }
        else if (compNameRad.IsChecked)
        {
            return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
        }
        else if (compTownRad.IsChecked)
        {
            return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
        }
        else if (compPcodeRad.IsChecked)
        {
            return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
        }
        return false;
    };
}
\$\endgroup\$
  • \$\begingroup\$ thanks for your answer. After implementing the code there is still a noticeable delay, especially when deleting characters from the search box. The old search method had no delay in filtering at all, I'm just wondering if I'm missing something obvious in why it is so slow now. \$\endgroup\$ – CBR Oct 19 '15 at 8:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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