1
votes

More about the question

First, let me clear the air... I'm somewhat new to C#. I bumped into this issue in an application I was working on. This particular class only had ObservableCollections exposed as Properties so my initial thought was that ObservervableCollection has it's own event, so I don't need the PropertyChanged event. The first attempt worked beautifully. Then I started cleaning up my code and found I didn't really need one of the backing vars, so I moved it all into the method...and it quit updating UI. Adding the INotifyPropertyChanged fixed the issue but I was left with a very big "WHY?"

Here is some test code I put together to see what I could figure out. Be advised, it is littered with bad practice, but I am really only trying to figure out when CollectionChanged can be depended on and when PropertyChagned must be added. Should I ever depend on CollectionChanged over PropertyChanged? If not, should we be using another List type, as we wouldn't really need the overhead of the ObservableCollection. It really seems like a waste to use both.

The bulk of the code

private TestClass test = new TestClass();

        public MainWindow()
        {
            InitializeComponent();
            this.DataContext = test;
        }

        internal class TestClass : INotifyPropertyChanged
        {
            public ObservableCollection<String> test1 = new ObservableCollection<String>();
            public ObservableCollection<String> test2 = new ObservableCollection<String>() { "T2-A" };

            public TestClass()
            {
            }

            public ObservableCollection<String> Test1 { get => test1; set { } }
            public ObservableCollection<String> Test2 { get => test2; set { this.test2 = value; } }
            public ObservableCollection<String> Test3 { get; set; }

            public event PropertyChangedEventHandler PropertyChanged;

            public void OnPropertyChanged(string propertyName = null) => PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

        private void BT1_Click(object sender, RoutedEventArgs e)
        {
            //Both of these work fine
            test.test1.Add("T1-A");
            test.Test1.Add("T1-B");

            //First line works...Second line breaks it... but there is a setter.
            test.Test2.Add("T2-C");
            test.Test2 = new ObservableCollection<String>() { "T2-A", "T2-B" };
            test.test2.Add("T2-D");

            //First Line throws Null exception... So it's not creating a backing var of it's own.
            //test.Test3.Add("T3-A");
            var t3 = new ObservableCollection<String>() { "T3-B", "T3-C" };
            test.Test3 = t3;
            test.Test3.Add("T3-D");
            //It updates when I trigger the property changed...but what broke CollectionChanged
            test.OnPropertyChanged(nameof(test.Test3));
        }

Here is the XAML if anyone is interested

<ListBox ItemsSource="{Binding Test1}" HorizontalAlignment="Left" Height="205" VerticalAlignment="Top" Width="161" Margin="10,5,0,0" />
        <ListBox ItemsSource="{Binding Test2}" Height="205" Margin="186,5,187,0" VerticalAlignment="Top" />
        <ListBox ItemsSource="{Binding Test3}" Height="205" Margin="366,5,8,0" VerticalAlignment="Top" />
        <Button x:Name="bT1" Content="Test It" HorizontalAlignment="Left" Height="33" Margin="10,215,0,0" VerticalAlignment="Top" Width="516" Click="BT1_Click" />

My Findings so far

It seems that if you want to use the ObservableCollection CollectionChanged event, you must create your backing var, and never alter the instance of Collection. In otherwords, use Clear() to wipe and rebuild rather than 'var col = new ObservableCollection()'. Is there something I am missing? I would think this would get rather flaky when you start looking at TwoWay data. How would you prevent someone from breaking your code downstream following the 2nd line of test2?

1
You are not missing anything. Never give this property a public setter, that is an accident waiting to happen. That is in general true for any property that exposes a collection object, this one rubs it in nicely. If it is necessary for external code to determine the collection object to be used then it should be passed in through the constructor so it can be set only once. Declare the backing field readonly to nail it down.Hans Passant

1 Answers

1
votes

//First line works...Second line breaks it... but there is a setter.

test.Test2.Add("T2-C");
test.Test2 = new ObservableCollection<String>() { "T2-A", "T2-B" };

Well, lets look at the setter

public ObservableCollection<String> Test2 { get => test2; set { this.test2 = value; } }

That does set the value to the new collection, but what about the binding? In order for the binding to know of the new object you need to raise the NotifyPropertyChanged event. So after you set that value currently the binding will still be pointing to the old collection (which still is listening for collection changes but on the previous value). Adding a notify property changed call to the setter will allow the binding to be updated:

private ObservableCollection<String> test2;
public ObservableCollection<String> Test2 
{ 
    get
    {
        return test2;
    } 
    set 
    { 
        test2 = value; 
        OnPropertyChanged("Test2");
    } 
}

Add this example to each property and your bindings will update when you update the entire collection to a new object. i.e. property = new ObservableCollection... Then the ListBox will be looking for collection changes on the new object.

In response to your last paragraph, there is another way to go about it:

You can keep one instance of the collection and never change it. i.e.:

public ObservableCollection<String> Test2 { get; set; }

Then in the constructor of the class initialize your collection. (or initialize it in-line, either works, the key is to only initialize it one time)

Test2 = new ObservableCollection<String>();

Then, when you want to create a new list do not overwrite the object. Instead just clear the list and add the new values in:

public void UpdateCollection(List<String> newValues)
{
    Test2.Clear(); //notifies the list box with the CollectionChanged event
    foreach(var value in newValues)
    {
        Test2.Add(value); //notifies the list box with the new item in the collection
    }
}