6
\$\begingroup\$

In my previous question, I got heaps of good advice BUT no solution for the problem apposed.

I applied the valuable learnings from the past on this.question and am also hitting it from a different angle.

The second part is the reason I didn't mark the question as 2.0 version of the first but feel free to let me know how to fix the overall CollectionView wiring.

The main issue here is speed, speed & speed. Everything is behaving and lightning fast until I add grouping functionality to the ListView's CollectionView and from saying that I mean the list takes way longer to load and after it does, functionality(s) such as filtering and searching (filtering) are not usable anymore. I believe this is partially due to using Refresh() as Dr.WPF points out:

Unfortunately, the Refresh() method results in a complete regeneration of the view. It is a rather drastic operation, to say the least. Furthermore, when a Refresh() occurs within the view, it raises a CollectionChanged notification and supplies the Action as “Reset”.

And also grouping itself as @scobi says in response to this SO answer,

Note that VirtualizingStackPanels are the default items panel template for ListViews. Using some features like grouping will override the default, however.

which all of a sudden it becomes clear why grouping bogs my ListView down.

Now I know what's the problem (I think), and looking for a solution to improve the code and speed it up.

AllPartsViewModel.cs

public class AllPartsViewModel : WorkspaceViewModel
{
    #region Fields

    readonly RelayCommand _clearSearch;
    readonly PartRepository _partRepository;
    readonly VendorRepository _vendorRepository;
    bool _errorParts; 
    private bool _priceSurgeMedium;
    private bool _priceSurgeHigh;
    List<EditPartViewModel> allpvms;
    RelayCommand _exportToCsv;
    INotifyCollectionChanged notifyCollectionChanged;
    string _searchFilterString;

    #endregion // Fields

    #region Constructor

    public AllPartsViewModel(PartRepository partRepository, VendorRepository vendorRepostiory)
    {
        if (partRepository == null)
            throw new ArgumentNullException("partRepository");

        _clearSearch = new RelayCommand(
                                        param => this.OnRequestCleanSearch(),
                                        param => this.CanClear
                                        );

        base.DisplayName = Strings.AllPartsViewModel_DisplayName;

        _partRepository = partRepository;
        _vendorRepository = vendorRepostiory;
        _searchFilterString = string.Empty;

        // Populates the AllParts collection with PartViewModels.
        this.CreateAllParts();
    }

    void CreateAllParts()
    {
        // GetParts() returns List<Part>. Now we can pour it's results 
        // using the following select in List<EditPartViewModel> on the fly.
        // allpvms: All Part View Models.
        allpvms = _partRepository.GetParts().Select(p => new EditPartViewModel(p, _vendorRepository)).ToList();

        foreach (EditPartViewModel cvm in allpvms)
        {
            cvm.PropertyChanged += this.OnPartViewModelPropertyChanged;
        }

        this.AllParts = new ObservableCollection<EditPartViewModel>(allpvms);
        this.AllParts.CollectionChanged += this.OnCollectionChanged;

        // We could use the ICollectionView interface rather. However it wouldn't have the list features
        // such as add/remove/count etc. 
        this.AllPartsCollectionView = (ListCollectionView)CollectionViewSource.GetDefaultView(this.AllParts);

        // Filtering
        AllPartsCollectionView.Filter = PartsFilter;

        //// Sorting 
        AllPartsCollectionView.SortDescriptions.Add(new SortDescription("PartName", ListSortDirection.Ascending));

        // Grouping
        AllPartsCollectionView.GroupDescriptions.Add(new PropertyGroupDescription("PartVendor"));

        // ListCollectionView explicitly implements INotifyCollectionChanged.CollectionChanged
        // from CollectionView. 
        // So to be able to bind to ListCollectionView's Count property, you need to cast
        // your collection view to INotifyCollectionChanged first. Then subscribe to 
        // CollectionChanged event such as below.
        notifyCollectionChanged = (INotifyCollectionChanged)this.AllPartsCollectionView;
        notifyCollectionChanged.CollectionChanged += OnNotifyCollectionChanged;
    }

    #endregion // Constructor

    #region Public Interface

    public ObservableCollection<EditPartViewModel> AllParts { get; set; }

    public ListCollectionView AllPartsCollectionView { get; private set; }

    public int CollectionCount { get { return AllPartsCollectionView.Count; } }

    #endregion // Public Interface

    #region Event Handling Methods

    private bool PartsFilter(Object item)
    {
        var editPartViewModel = item as EditPartViewModel;
        return PartsSearchFilter(editPartViewModel) &&
               PartsWithErrorFilter(editPartViewModel) &&
               PartsWithPriceSurgeMedium(editPartViewModel) &&
               PartsWithPriceSurgeHigh(editPartViewModel);
    }

    private bool PartsSearchFilter(EditPartViewModel editPartViewModel)
    {
        // Rather than changing both sides to ToUpper() which leads to 
        // creating a new string in each attempt, I used IndexOf().
        return editPartViewModel.PartName.IndexOf(_searchFilterString, StringComparison.OrdinalIgnoreCase) >= 0;
    }

    private bool PartsWithErrorFilter(EditPartViewModel editPartViewModel)
    {
        return _errorParts ? editPartViewModel.HasErrors : true;
    }

    private bool PartsWithPriceSurgeHigh(EditPartViewModel editPartViewModel)
    {
        return _priceSurgeHigh ? editPartViewModel.PriceSurgeLevel == 3 : true;
    }

    private bool PartsWithPriceSurgeMedium(EditPartViewModel editPartViewModel)
    {
        return _priceSurgeMedium ? editPartViewModel.PriceSurgeLevel == 2 : true;
    }

    // This is for the AllPartsCollectionView.
    private void OnNotifyCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        OnPropertyChanged("CollectionCount");
    }

    // This is for AllParts.
    void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        if (e.NewItems != null && e.NewItems.Count != 0)
            foreach (EditPartViewModel partVM in e.NewItems)
                partVM.PropertyChanged += this.OnPartViewModelPropertyChanged;

        if (e.OldItems != null && e.OldItems.Count != 0)
            foreach (EditPartViewModel partVM in e.OldItems)
                partVM.PropertyChanged -= this.OnPartViewModelPropertyChanged;
    }

    void OnPartViewModelPropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        string IsSelected = "IsSelected";

        // Makes sure that the property name we're referencing is valid.
        // This is a debugging technique, and doesn't execute in a release build.
        (sender as EditPartViewModel).VerifyPropertyName(IsSelected);
    }

    #endregion // Event Handling Methods

    #region Event Handlers

    // Command bound to 'Export the List' button in the view.
    public ICommand ExportToCsv
    {
        get
        {
            if (_exportToCsv == null)
            {
                _exportToCsv = new RelayCommand(
                    param => this.OnRequestExportToCsv());
            }
            return _exportToCsv;
        }
    }

    public ICommand ClearSearch { get { return _clearSearch; } }

    public bool ShowErrorsOnly
    {
        get { return _errorParts; }
        set
        {
            _errorParts = value;
            OnPropertyChanged("ShowErrorsOnly");
            AllPartsCollectionView.Refresh();
        }
    }

    public bool ShowHighPriceSurgeOnly
    {
        get { return _priceSurgeHigh; }
        set
        {
            _priceSurgeHigh = value;
            OnPropertyChanged("ShowHighPriceSurgeOnly");
            AllPartsCollectionView.Refresh();
        }
    }

    public bool ShowMediumPriceSurgeOnly
    {
        get { return _priceSurgeMedium; }
        set
        {
            _priceSurgeMedium = value;
            OnPropertyChanged("ShowMediumPriceSurgeOnly");
            AllPartsCollectionView.Refresh();
        }
    }

    public string SearchFilterString
    {
        get { return _searchFilterString; }
        set
        {
            _searchFilterString = value;
            OnPropertyChanged("SearchFilterString");
            AllPartsCollectionView.Refresh();
        }
    }

    #endregion // Event Handlers

    #region Private Helpers

    private void OnRequestCleanSearch()
    {
        SearchFilterString = string.Empty;
    }

    private void OnRequestExportToCsv()
    {
        ///...
    }

    // returns true if the searchFilterString isn't empty.
    bool CanClear
    {
        get
        {
            return !string.IsNullOrEmpty(this._searchFilterString);
        }
    }

    #endregion // Private Helpers

    #region Base Class Overrides

    protected override void OnDispose()
    {
        // TODO: Added '.ToList()' to the end of AllParts is a change
        //      made to fix the 'Collection was modified; enumeration 
        //      operation may not execute' error. Investigate the 
        //      effects of this change.
        foreach (EditPartViewModel partVM in this.AllParts.ToList())
        {
            partVM.Dispose();

            this.AllParts.Clear();
            this.AllParts.CollectionChanged -= this.OnCollectionChanged;
        }
        // To release the handler delegated to this event.
        this.notifyCollectionChanged.CollectionChanged -= this.OnNotifyCollectionChanged;
    }

    #endregion // Base Class Overrides

}

AllPartsView.xaml (partial)

<ListView
  AlternationCount="2"
  d:DataContext="{d:DesignInstance Type=vm:MockViewModel, IsDesignTimeCreatable=True}"
  ItemsSource="{Binding AllPartsCollectionView}"
  >
  <ListView.GroupStyle>
    <!-- The moment the tags below added, ListView
         becomes unsuable due to low speed-->
    <GroupStyle>
      <GroupStyle.ContainerStyle>
        <Style TargetType="{x:Type GroupItem}">
          <Setter Property="Template">
            <Setter.Value>
              <ControlTemplate>
                <Expander Header="{Binding Path=PartVendor}" IsExpanded="True">
                  <ItemsPresenter />
                </Expander>
              </ControlTemplate>
            </Setter.Value>
          </Setter>
        </Style>
      </GroupStyle.ContainerStyle>
    </GroupStyle>
  </ListView.GroupStyle>
  <ListView.ItemContainerStyle>
    <Style TargetType="{x:Type ListViewItem}">
      <EventSetter Event="MouseDoubleClick" Handler="ListViewItem_MouseDoubleClick" />
      <Setter Property="HorizontalContentAlignment" Value="Stretch"/>
    </Style>
  </ListView.ItemContainerStyle>
</ListView>

I did a fair amount of research on this and read couple of really interesting articles including

However in most of these approaches the CollectionViewSource is applied within the XAML using the tag below

<CollectionViewSource x:Key="cvsActors" Source="{Binding ActorList}" >
    ...
</CollectionViewSource>

rather than in the code (ViewModel) the way I did it

this.AllPartsCollectionView = (ListCollectionView)CollectionViewSource.GetDefaultView(this.AllParts);

which helps me apply the filtering and all in a much easier way. However, if switching to the method above is the fix I have no problem doing so.

Limitation

I am also limited to the .NET 4.0 which prevents me from using LiveFiltering & LiveSortingProperties tags such as below:

<Window.Resources>
    <CollectionViewSource x:Key="cvsActors" Source="{Binding ActorList}"
                          IsLiveSortingRequested="True">
        <CollectionViewSource.LiveSortingProperties>
            <clr:String>LastName</clr:String>
        </CollectionViewSource.LiveSortingProperties>
        <CollectionViewSource.SortDescriptions>
            <scm:SortDescription PropertyName="LastName" />
        </CollectionViewSource.SortDescriptions>
    </CollectionViewSource>
</Window.Resources>

and I am not doing any editing on the items (yet), then I don't know how switching to IEditibleCollectionView would help.

You can see the screenshot of what I have created so far in the previous question (mentioned in the first paragraph) and I should say in this stage this code is fully functional in the way it's represented in this question and the only issue is the speed when I add the <ListView.GroupStyle> tag to my ListView.

Please let me know what you think about this problem and please don't hold back any ideas as I am happy to strip the whole thing down and take a different approach if it's in any way will be a better-practice or even solve current and future issues.

\$\endgroup\$
6
\$\begingroup\$

I have extensively studied this issue during the past week and this is the result for how to improve the speed in the ListView when Grouping is activated.

In a normal WPF ItemsControl that virtualizes, such as ListBox or ListView, virtualization turns off when you turn grouping on.

And using .NET 4.0, I was unable to find any reasonable solution other than some really messy ones like this or even using TreeListView instead.

In the higher versions of .NET such as 4.5 there is a property introduced called VirtualizingPanel.IsVirtualizingWhenGrouping which might be able to fix the performance issue of the ListView. And I believe it is more than possible as the MSDN description says:

Gets or sets a value that indicates whether this VirtualizingPanel virtualizes the items in its collection when it displays groups.

So the final conclusion is that, if you're using .NET 4 and lower, use grouping only with the small amount of members to manage. If you go data in thousands or even in hundreds range, you'd be looking at non standard alternatives or simply not using the grouping feature.

Update

I have recently bumped up my .NET version from 4.0 to 4.5 and following that I used VirtualizingPanel.IsVirtualizingWhenGrouping as shown in the code below and the results are amazing. Grouping works very smooth and other features such as search and filtering won't get effected at all.

<ListView
  ItemsSource="{Binding AllPartsCollectionView}"
  VirtualizingPanel.IsVirtualizingWhenGrouping="True"
  >
  <ListView.GroupStyle>
    <GroupStyle>
      <GroupStyle.ContainerStyle>
        <Style TargetType="{x:Type GroupItem}">
          <Setter Property="Template">
            <Setter.Value>
              <ControlTemplate>
                <!--The grouping binding should always be "Name". Bazaar!!!-->
                <Expander Header="{Binding Path=Name}" IsExpanded="True">
                  <ItemsPresenter />
                </Expander>
              </ControlTemplate>
            </Setter.Value>
          </Setter>
        </Style>
      </GroupStyle.ContainerStyle>
    </GroupStyle>
  </ListView.GroupStyle
 ...
</ListView
\$\endgroup\$
4
\$\begingroup\$
#region Fields

readonly RelayCommand _clearSearch;
readonly PartRepository _partRepository;
readonly VendorRepository _vendorRepository;
bool _errorParts; 
private bool _priceSurgeMedium;
private bool _priceSurgeHigh;
List<EditPartViewModel> allpvms;
RelayCommand _exportToCsv;
INotifyCollectionChanged notifyCollectionChanged;
string _searchFilterString;

#endregion // Fields

A few things stick out:

readonly RelayCommand _clearSearch;
readonly PartRepository _partRepository;
readonly VendorRepository _vendorRepository;

Nice. I like readonly private fields. Very much. And I like the underscore prefix, but that's just my own personal preference - some others may prefer seeing a this. qualifier everywhere a field is referenced.

bool _errorParts; 

Less cool. _errorParts sounds very, very much like an IEnumerable<ErrorPart>... and it's not. There's a convention in C# to prefix Boolean fields with is or has, and bool methods/properties with Is or Has.

private bool _priceSurgeMedium;
private bool _priceSurgeHigh;

Same thing here... now all these fields are private unless specified otherwise - by explicitly marking these two as private, you're making me wonder whether the other fields were intended to be public, or even thought to be implicitly public... in any case a public field is a missed encapsulation opportunity. Bottom line here, consistency matters: either make them all implicitly private, or all explicitly private.

List<EditPartViewModel> allpvms;
INotifyCollectionChanged notifyCollectionChanged;

Where's the underscore prefix? Again, it's not required, but since all other fields have it... why not these two?

#region Fields

Clutter. There's no need to wrap code in #region blocks; these things are for auto-generated code. Any maintainer can see that these are fields, that a constructor is a constructor, that private methods are private, that overrides override a base method, etc.

#endregion // Fields

Bad, useless comment on an already fluffy #region block. A region is a comment... this is like commenting on a comment, it's ...a meta-comment! Remove fearlessly.

In fact, as I scoll down the class, I can see a lot of redundant comments. Some examples:

    // Filtering
    AllPartsCollectionView.Filter = PartsFilter;

    //// Sorting 
    AllPartsCollectionView.SortDescriptions.Add(new SortDescription("PartName", ListSortDirection.Ascending));

    // Grouping
    AllPartsCollectionView.GroupDescriptions.Add(new PropertyGroupDescription("PartVendor"));
\$\endgroup\$
  • \$\begingroup\$ I have gone through the variables and did the changes you pointed out. I gotta say, now it's way better. \$\endgroup\$ – Mehrad Jan 16 '15 at 5:59

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.