Wiring up CollectionChanged and PropertyChanged (O

Question

WPF DataBindings used to make me happy. One thing I've stumbled over just now is that at some point they just don't refresh as intented. Please take a look at the following (fairly simple) code:

<Window x:Class="CVFix.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    Title="MainWindow" Height="350" Width="300">
  <Grid>
    <Grid.ColumnDefinitions>
        <ColumnDefinition></ColumnDefinition>
    </Grid.ColumnDefinitions>
    <Grid.RowDefinitions>
        <RowDefinition></RowDefinition>
        <RowDefinition Height="40"></RowDefinition>
    </Grid.RowDefinitions>
    <ListBox Grid.Row="0" Grid.Column="0" 
                  ItemsSource="{Binding Path=Persons}"
                  SelectedItem="{Binding Path=SelectedPerson}"
                  x:Name="lbPersons"></ListBox>
    <TextBox Grid.Row="1" Grid.Column="0" Text="{Binding Path=SelectedPerson.Name, UpdateSourceTrigger=PropertyChanged}"/>
  </Grid>
</Window>

The Code behind for the XAML:

using System.Windows;
namespace CVFix
{
  /// <summary>
  /// Interaction logic for MainWindow.xaml
  /// </summary>
  public partial class MainWindow : Window
  {
    public ViewModel Model { get; set; }

    public MainWindow()
    {
        InitializeComponent();
        this.Model = new ViewModel();
        this.DataContext = this.Model;
    }
  }
}

Finally, here's the ViewModel classes:

using System.Collections.ObjectModel;
using System.ComponentModel;

namespace CVFix
{
  public class ViewModel : INotifyPropertyChanged
  {
    private PersonViewModel selectedPerson;

    public PersonViewModel SelectedPerson
    {
        get { return this.selectedPerson; }
        set
        {
            this.selectedPerson = value;

            if (this.PropertyChanged != null)
                this.PropertyChanged(this, new PropertyChangedEventArgs("SelectedPerson"));
        }
    }

    public ObservableCollection<PersonViewModel> Persons { get; set; }

    public ViewModel()
    {
        this.Persons = new ObservableCollection<PersonViewModel>();
        this.Persons.Add(new PersonViewModel() { Name = "Adam" });
        this.Persons.Add(new PersonViewModel() { Name = "Bobby" });
        this.Persons.Add(new PersonViewModel() { Name = "Charles" });
    }

    public event PropertyChangedEventHandler PropertyChanged;
  }
}

public class PersonViewModel : INotifyPropertyChanged
{
    private string name;

    public string Name
    {
        get { return this.name; }
        set
        {
            this.name = value;
            if(this.PropertyChanged != null)
            this.PropertyChanged(this, new PropertyChangedEventArgs("Name"));
        }
    }

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

    public event PropertyChangedEventHandler PropertyChanged;
}

What I'd want to happen: As I select an entry from the ListBox and modify its Name in the TextBox, the list gets updated to display the new value.

What happens : nothing. And that is the correct behaviour, If I'm any judge. I made sure that the SelectedItem's PropertyChanged is fired, but that does (of course) not cause CollectionChanged to be fired.

To fix this, I created an ObservableCollection-derived class that has a public OnCollectionChanged method, see here :

public class PersonList : ObservableCollection<PersonViewModel>
{
    public void OnCollectionChanged()
    {
        this.OnCollectionChanged(new NotifyCollectionChangedEventArgs( NotifyCollectionChangedAction.Reset ));
    }
}

I access this from the ViewModel's constructor, as described below:

    public ViewModel()
    {
        PersonViewModel vm1 = new PersonViewModel()
        {
            Name = "Adam"
        };
        PersonViewModel vm2 = new PersonViewModel()
        {
            Name = "Bobby"
        };
        PersonViewModel vm3 = new PersonViewModel()
        {
            Name = "Charles"
        };
        vm1.PropertyChanged += this.PersonChanged;

        this.Persons = new PersonList();


        this.Persons.Add(vm1);
        this.Persons.Add(vm2);
        this.Persons.Add(vm3);
    }

    void PersonChanged(object sender, PropertyChangedEventArgs e)
    {
        this.Persons.OnCollectionChanged();
    }

It works, but it's not a clean solution. My next idea would be creating a derivate of ObservableCollection that does the wiring up automatically in a CollectionChanged-handler.

public class SynchronizedObservableCollection<T> : ObservableCollection<T> where T : INotifyPropertyChanged
{
    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        switch (e.Action)
        {
            case NotifyCollectionChangedAction.Add:
                {
                    foreach (INotifyPropertyChanged item in e.NewItems)
                    {
                        item.PropertyChanged += this.ItemChanged;
                    }
                    break;
                }

            case NotifyCollectionChangedAction.Remove:
                {
                    foreach (INotifyPropertyChanged item in e.OldItems)
                    {
                        item.PropertyChanged -= this.ItemChanged;
                    }
                    break;
                }
        }
        base.OnCollectionChanged(e);
    }

    void ItemChanged(object sender, PropertyChangedEventArgs e)
    {
        this.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
    }
}

The question is : is there a better way to do this? Is this really necessary?

Thanks a lot in advance for any input!

Answer 1

No, it's not necessary at all. The reason your sample is failing is subtle, but quite simple.

If you don't provide WPF with a template for a data item (such as the Person objects in your list), it'll default to using the ToString() method to display. That's a member, not a property, and so you get no event notification when the value changes.

If you add DisplayMemberPath="Name" to your listbox, it'll generate a template that binds properly to the Name of your person - which will then update automatically as you'd expect.

Answer 2

I believe this is to do with your ToString() override on PersonViewModel. If you remove this, and use a DataTemplate on the ListBox instead, then you should get your expected behaviour:

<Window x:Class="CVFix.MainWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
Title="MainWindow" Height="350" Width="300">
<Grid>
    <Grid.ColumnDefinitions>
        <ColumnDefinition></ColumnDefinition>
    </Grid.ColumnDefinitions>
    <Grid.RowDefinitions>
        <RowDefinition></RowDefinition>
        <RowDefinition Height="40"></RowDefinition>
    </Grid.RowDefinitions>
    <ListBox Grid.Row="0" Grid.Column="0" 
              ItemsSource="{Binding Path=Persons}"
              SelectedItem="{Binding Path=SelectedPerson}"
              x:Name="lbPersons">
        <ListBox.ItemTemplate>
            <DataTemplate>
                <TextBlock Text="{Binding Name}" />
            </DataTemplate>
        </ListBox.ItemTemplate>            
    </ListBox>
    <TextBox Grid.Row="1" Grid.Column="0" Text="{Binding Path=SelectedPerson.Name, UpdateSourceTrigger=PropertyChanged}"/>
</Grid>

Answer 3

add DisplayMemberPath="Name" to ListBox. The problem is you are relying on ToString() to display person's name and not any property. That is why raising PropertyChanged does not make any difference. From now on, don't use a method to evaluate any value in Bindings.