1
\$\begingroup\$

I have a DataGrid that I filter when the user inputs text into a search box. The column that is filtered is based upon which radio button the user selects (Name, Town or Post Code). Although initially nothing happens until the user has started typing into the searchbox, for some reason there is a noticeable delay in selecting the different radio buttons.

Intuitively I thought that something was happening when the radio button is clicked, but there is (as far as I can tell) nothing being called. The C# program is relatively long, so I'll include the XAML for the radio buttons and the methods that they are being used but not the whole program, unless nothing is found originally.

SearchGrid

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

                if (compNameRad.IsChecked == true)
                {
                    return compDetails.CompanyName != null && CharactersOnly(compDetails.CompanyName.ToLower()).Contains(searchText);
                }
                if (compTownRad.IsChecked == true)
                {
                    return compDetails.CompanyTown != null && CharactersOnly(compDetails.CompanyTown.ToLower()).Contains(searchText);
                }
                if (compPcodeRad.IsChecked == true)
                {
                    return compDetails.CompanyPcode != null && CharactersOnly(compDetails.CompanyPcode.ToLower()).Contains(searchText);
                }
                return false;
            });
        }
        else
        {
            FillDataGrid();
        }
    }

ClearFilter

    private void ClearFilter(object sender, KeyEventArgs e)
    {
        if (e.Key == Key.Escape)
        {
            searchBox.Text = string.Empty;
            compNameRad.IsChecked = false;
            compTownRad.IsChecked = false;
            compPcodeRad.IsChecked = false;
            FillDataGrid();
        }
    }

XAML

                <StackPanel Margin="0" Grid.Row="1">
                    <Label Content="Search" HorizontalAlignment="Center" Margin="0,10,0,5" VerticalAlignment="Center" FontSize="21.333" FontWeight="Bold"/>
                    <RadioButton x:Name="compNameRad" Content="Name" HorizontalAlignment="Left" VerticalAlignment="Top" FontSize="18.667" GroupName="RadSearch" Margin="7,0,0,5"/>
                    <RadioButton x:Name="compTownRad" Content="Town" HorizontalAlignment="Left" VerticalAlignment="Top" FontSize="18.667" GroupName="RadSearch" Margin="7,5,0,5"/>
                    <RadioButton x:Name="compPcodeRad" Content="Pcode" HorizontalAlignment="Left" Margin="7,5,0,5" VerticalAlignment="Top" FontSize="18.667" GroupName="RadSearch"/>
                    <TextBox x:Name="searchBox" TextWrapping="Wrap" Margin="20,5" FontSize="18.667" VerticalAlignment="Center" TextChanged="SearchGrid"/>
                    <Label Content="Press Esc to clear filter" Margin="5,10,5,5" FontSize="16"/>
                </StackPanel>

I really see no reason for anything to slow down purely based on the selection of the radio button, is there a way to improve this code?

\$\endgroup\$
5
  • 1
    \$\begingroup\$ Do not use ToLower when comparing string, do it properly by using String.IndexOf Method (String, StringComparison). \$\endgroup\$ – BCdotWEB Oct 23 '15 at 12:25
  • \$\begingroup\$ Thanks for the comments chaps. When I do if (compNameRad.IsChecked) I get this error: Cannot implicitly convert type 'bool?' to 'bool'. An explicit conversion exists (are you missing a cast?) \$\endgroup\$ – CBR Oct 23 '15 at 13:46
  • 1
    \$\begingroup\$ @CBR ah, IsChecked it's nullable. Sorry then. I retract my comment, it was incorrect. \$\endgroup\$ – Konrad Morawski Oct 23 '15 at 13:47
  • \$\begingroup\$ @KonradMorawski, if I cast it to a bool it works. What would you suggest is the better method? \$\endgroup\$ – CBR Oct 23 '15 at 13:55
  • \$\begingroup\$ wouldn't this crash on a null? I'd say either == true or ?? false, see stackoverflow.com/questions/2673918/… \$\endgroup\$ – Konrad Morawski Oct 23 '15 at 14:24
1
\$\begingroup\$

I believe the GetDefaultView method is the bottle neck. If you refer to the source code you'll see that there are serverals loops involved in getting the view:

You start here:

public static ICollectionView GetDefaultView(object source)
{
    return GetOriginalView(GetDefaultCollectionView(source, true));
}

it calls:

static internal CollectionView GetDefaultCollectionView(object source, bool createView, Func<object, object> GetSourceItem=null)
{
    if (!IsValidSourceForView(source))
        return null;

    DataBindEngine engine = DataBindEngine.CurrentDataBindEngine;
    ViewRecord viewRecord = engine.GetViewRecord(source, DefaultSource, null, createView, GetSourceItem);

    return (viewRecord != null) ? (CollectionView)viewRecord.View : null;
}

it then calls the GetViewRecord which seems to be really heavy:

internal ViewRecord GetViewRecord(object collection, CollectionViewSource cvs, Type collectionViewType, bool createView, Func<object, object> GetSourceItem)
{
    // Order of precendence in acquiring the View:
    // 0) If  collection is already a CollectionView, return it.
    // 1) If the CollectionView for this collection has been cached, then
    //    return the cached instance.
    // 2) If a CollectionView derived type has been passed in collectionViewType
    //    create an instance of that Type
    // 3) If the collection is an ICollectionViewFactory use ICVF.CreateView()
    //    from the collection
    // 4) If the collection is an IListSource call GetList() and perform 5),
    //    etc. on the returned list
    // 5) If the collection is an IBindingList return a new BindingListCollectionView
    // 6) If the collection is an IList return a new ListCollectionView
    // 7) If the collection is an IEnumerable, return a new CollectionView
    //    (it uses the ListEnumerable wrapper)
    // 8) return null
    // An IListSource must share the view with its underlying list.

    // if the view already exists, just return it
    // Also, return null if it doesn't exist and we're called in "lazy" mode
    ViewRecord viewRecord = GetExistingView(collection, cvs, collectionViewType, GetSourceItem);
    if (viewRecord != null || !createView)
    {
        return viewRecord;
    }

    // If the collection is an IListSource, it uses the same view as its
    // underlying list.
    IListSource ils = collection as IListSource;
    IList ilsList = null;
    if (ils != null)
    {
        ilsList = ils.GetList();
        viewRecord = GetExistingView(ilsList, cvs, collectionViewType, GetSourceItem);

        if (viewRecord != null)
        {
            return CacheView(collection, cvs, (CollectionView)viewRecord.View, viewRecord);
        }
    }

    // Create a new view
    ICollectionView icv = collection as ICollectionView;

    if (icv != null)
    {
        icv = new CollectionViewProxy(icv);
    }
    else if (collectionViewType == null)
    {
        // Caller didn't specify a type for the view.
        ICollectionViewFactory icvf = collection as ICollectionViewFactory;
        if (icvf != null)
        {
            // collection is a view factory - call its factory method
            icv = icvf.CreateView();
        }
        else
        {
            // collection is not a factory - create an appropriate view
            IList il = (ilsList != null) ? ilsList : collection as IList;
            if (il != null)
            {
                // create a view on an IList or IBindingList
                IBindingList ibl = il as IBindingList;
                if (ibl != null)
                    icv = new BindingListCollectionView(ibl);
                else
                    icv = new ListCollectionView(il);
            }
            else
            {
                // collection is not IList, wrap it
                IEnumerable ie = collection as IEnumerable;
                if (ie != null)
                {
                    icv = new EnumerableCollectionView(ie);
                }
            }
        }
    }
    else
    {
        // caller specified a type for the view.  Try to honor it.
        if (!typeof(ICollectionView).IsAssignableFrom(collectionViewType))
            throw new ArgumentException(SR.Get(SRID.CollectionView_WrongType, collectionViewType.Name));

        // if collection is IListSource, get its list first (bug 1023903)
        object arg = (ilsList != null) ? ilsList : collection;

        try
        {
            icv = Activator.CreateInstance(collectionViewType,
                            System.Reflection.BindingFlags.CreateInstance, null,
                            new object[1]{arg}, null) as ICollectionView;
        }
        catch (MissingMethodException e)
        {
            throw new ArgumentException(SR.Get(SRID.CollectionView_ViewTypeInsufficient,
                            collectionViewType.Name, collection.GetType()), e);
        }
    }

    // if we got a view, add it to the tables
    if (icv != null)
    {
        // if the view doesn't derive from CollectionView, create a proxy that does
        CollectionView cv = icv as CollectionView;
        if (cv == null)
            cv = new CollectionViewProxy(icv);

        if (ilsList != null)    // IListSource's list shares the same view
            viewRecord = CacheView(ilsList, cvs, cv, null);

        viewRecord = CacheView(collection, cvs, cv, viewRecord);

        // raise the event for a new view
        BindingOperations.OnCollectionViewRegistering(cv);
    }

    return viewRecord;
}

who knows what eles it does :-)

then finally there is another loop involved here:

// return the original (un-proxied) view for the given view
static ICollectionView GetOriginalView(ICollectionView view)
{
    for (   CollectionViewProxy proxy = view as CollectionViewProxy;
            proxy != null;
            proxy = view as CollectionViewProxy)
    {
        view = proxy.ProxiedView;
    }

    return view;
}

Refactoring

I suggest you refactor your SearchGrid in a way that you retrieve the view somewhere else and here just update the filter:

private ICollectionView _view;

private void InitializeSearchView()
{
    view = CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
}

private void SearchGrid(object sender, TextChangedEventArgs e)
{
    if (string.IsNullOrEmpty(searchBox.Text))
    {    
        FillDataGrid();
        return;
    }    
    
    var searchText = CharactersOnly(searchBox.Text);
    _view.Filter = (obj =>
    {
        CompanyModel compDetails = obj as CompanyModel;
        if (compDetails == null)
        {
            return true;
        }

        if (compNameRad.IsChecked == true)
        {
            return CompanyContains(compDetails.CompanyName, searchText);
        }
        if (compTownRad.IsChecked == true)
        {
            return CompanyContains(compDetails.CompanyTown, searchText);
        }
        if (compPcodeRad.IsChecked == true)
        {
            return CompanyContains(compDetails.CompanyPcode, searchText);
        }
        return false;
    });
}

at the same time I'd move the condition into a dedicated method and remove nesting by inverting the above if:

private bool CompanyContains(string value, string searchText)
{
    return !string.IsNullOrEmpty(value) && CharactersOnly(value).IndexOf(searchText) > 0;
}
\$\endgroup\$

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.