ObservableCollection via ListCollectionView displays incorrect list of items

8.8k views Asked by At

C#:

public partial class MainWindow : Window
{
    private readonly ViewModel vm;

    public MainWindow()
    {
        InitializeComponent();
        vm = new ViewModel();

        DataContext = vm;
    }

    private void Button_Click(object sender, RoutedEventArgs e)
    {
        vm.Models.RemoveAt(0);
    }
}

public class ViewModel
{
    public ObservableCollection<Model> Models { get; set; }
    public ListCollectionView View { get; set; }

    public ViewModel()
    {
        Models = new ObservableCollection<Model>()
        {
            new Model() { Name = "Gordon Freeman" },
            new Model() { Name = "Isaac Kleiner" },
            new Model() { Name = "Eli Vance" },
            new Model() { Name = "Alyx Vance" },
        };

        Models.CollectionChanged += (s, e) => View.Refresh();
        View = new ListCollectionView(Models);
    }
}

public class Model
{
    public string Name { get; set; }

    public override string ToString()
    {
        return Name;
    }
}

XAML:

<StackPanel>
    <ListBox ItemsSource="{Binding Path=View}" />
    <Button Click="Button_Click">Click</Button>
</StackPanel>

The ObservableCollection contains 4 elements, and the ListBox is displaying all 4, as expected. When the button is clicked, the 1st element of the ObservableCollection is removed. The ListBox, however, is now displaying only the 2nd and 3rd. It would appear the 1st and 4th have been removed.

If the line Models.CollectionChanged += (s, e) => View.Refresh(); is moved after View = new ListCollectionView(Models); (or commented out entirely) things work as expected.

Why?

P.S. This is a simple piece of a larger puzzle. In this small example, I realize I don't need to call View.Refresh(); on CollectionChanged for the ListBox to update itself.

4

There are 4 answers

2
sayed saad On

Try not to use the new keyword to define your new instance of listCollectionView. Use the following instead.

var sortedCities  = CollectionViewSource.GetDefaultView(Models);
0
Catalin Serafimescu On

Seems to be a problem with ListView refresh. If you bind a labe/TextBlock to ListView.Items.Count, you'll see that the list still has 3 items (after first delete).

1
brunnerh On

My guess would be that the refresh interferes with the automatic update by the view. Presumably the view subscribes to CollectionChanged as well in the constructor, so if you also subscribe to the event before the view does and call refresh you get an unwanted update in between the collection change and the view's own update.

e.g.

Item 0 is removed -> Notify event listeners
=> Your handler: Refresh() -> Rebuild view => Item gets removed.
=> View handler: Event args say: Item X was removed -> Remove item X

This still does not explain why the first and the last items get removed but it seems reasonable to me.

If the subscription is after the view instantiation:

Item 0 is removed -> Notify event listeners
=> View handler: Event args say: Item X was removed -> Remove item X
=> Your handler: Refresh() -> Rebuild view => Nothing changed.

0
Adam Vincent On

Even though you're wrapping your Model class in an ObservableCollection, you still need to implement INotifyPropertyChanged for it. I've banged my head against this problem enough times that now I remember to start troubleshooting at the lowest building block, i.e. Model.

public class Model : INotifyPropertyChanged
{
    #region INotify Implementation
    public event PropertyChangedEventHandler PropertyChanged;
    private void NotifyPropertyChanged(String info)
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(info));
        }
    }
    #endregion

    public string _Name;
    public string Name {
        get { return _Name;  }
        set { _Name = value; NotifyPropertyChanged("Name"); }

    }
    public override string ToString()
    {
        return Name;
    }
}

Also, in your ViewModel, when INotifyPropertyChanged is implemented correctly you don't need the below lines. INotifyPropertyChanged will update your UI for you.

    Models.CollectionChanged += (s, e) => View.Refresh();
    View = new ListCollectionView(Models);