Two way data binding issue in a MVVM

Question

I am trying to build a simple C#/WinFoms project that uses the Model-View-ViewModel design pattern according to the structure:

The data binding between the two UserControls and the associated ViewModels does not work well.

The MainForm contains two UserControls (UC): Uc_Create, Uc_Iteration. Each UC contains a combobox that is connected to the associated property in the ViewModel_xxx, namely

Uc_Create has:

this.comboBox1ComplexCreate.DataSource = oVM_Create.VM_Create_ListOfStringsInModel; 

Uc_Iteration has:

this.comboBox1ComplexIteration.DataSource = oVM_Iteration.VM_Iteration_ListOfStringsInModel;

The problem:

When I add elements to VM_Iteration_ListOfStringsInModel the combobox in the corresponding UC (comboBox1ComplexCreate) and the list in the Model are correctly changed but the other combobox (comboBox1ComplexIteration) in Uc_Iteration is not!

Why????

If I change the List in the Model to a BindingList, everything works fine. What am I doing wrong?

Thanks in advance!

---

Model:

namespace Small_MVVM
{

    public class Model 
    {
        private static readonly object m_oLock = new object();

        private static Model instance;


        public List<string> simplelistOfStrings; 

        private Model()
        {
            simplelistOfStrings = new List<string>();
        }

        public static Model GetInstance()
        {
            if (instance == null)
            {
                lock (m_oLock)
                {
                    if (instance == null)
                    {
                        instance = new Model();
                    }
                }
            }
            return instance;
        }
    }
}

ModelView_Create:

namespace Small_MVVM
{
    class ViewModel_Create : NotifyPropertyChangedBase
    {
        private static Model oModel = Model.GetInstance();

        private BindingList<string> _VM_Create_ListOfStringsInModel = new BindingList<string>(oModel.simplelistOfStrings);
        public BindingList<string> VM_Create_ListOfStringsInModel
        {
            get
            {

                return _VM_Create_ListOfStringsInModel;
            }
            set
            {
                _VM_Create_ListOfStringsInModel = value;
                this.FirePropertyChanged(nameof(VM_Create_ListOfStringsInModel));
            }
        }
    }
}

ModelView_Iteration:

namespace Small_MVVM
{

    class ViewModel_Iteration : NotifyPropertyChangedBase
    {
        private static Model oModel = Model.GetInstance();

        public ViewModel_Iteration()
        {

        }

        BindingList<string> _VM_Iteration_ListOfStringsInModel = new BindingList<string>(oModel.simplelistOfStrings);
        public BindingList<string> VM_Iteration_ListOfStringsInModel
        {
            get
            {
                return _VM_Iteration_ListOfStringsInModel;
            }
            set
            {
                _VM_Iteration_ListOfStringsInModel = value;
                this.FirePropertyChanged(nameof(VM_Iteration_ListOfStringsInModel));
            }
        }
    }
}

This is the abstract class NotifyPropertyChangedBase that implements the INotifyPropertyChange interface:

public abstract class NotifyPropertyChangedBase : INotifyPropertyChanged
{

    public event PropertyChangedEventHandler PropertyChanged;

    protected bool CheckPropertyChanged<T>(string propertyName, ref T oldValue, ref T newValue)
    {
            if (oldValue == null && newValue == null)
        {
                return false;
        }

        if ((oldValue == null && newValue != null) || !oldValue.Equals((T)newValue))
        {
            oldValue = newValue;
            return true;
        }

        return false;
    }

    private delegate void PropertyChangedCallback(string propertyName);

    protected void FirePropertyChanged(string propertyName)
    {
         if (this.PropertyChanged != null)
        {
            this.PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

Answer 1

If you are following MVVM pattern , then you should make use of ObservableCollection for collections like as below

   private ObservableCollection<int> _intList;
   public ObservableCollection<int> IntList
     {
        get
        {
          return _intList;
        }
        set
        {
           _intList= value;
           _intList.CollectionChanged += 
                        new System.Collections.Specialized.NotifyCollectionChangedEventHandler
                                            (MyProperty_CollectionChanged);
            }
        } 

void MyProperty_CollectionChanged(object sender,                        
         System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
  {
     NotifyPropertyChanged("IntList");
 }

ObservableCollection - Represents a dynamic data collection that provides notifications when items get added, removed, or when the whole list is refreshed.

you can also check this : MVVM (Model-View-ViewModel) Pattern For Windows Form Applications, using C#

Answer 2

I think your problem is that there is no link between your two BindingLists.

When you add items to VM_Iteration_ListOfStringsInModel the changes are propagated to the bound combobox and the underlying List in your model. But the list in your model cannot further propagate these changes to VM_Create_ListOfStringsInModel as it does not support this feature. VM_Create_ListOfStringsInModel will contain the item you added to VM_Iteration_ListOfStringsInModel but it won't raise a ListChanged event as List in you model breaks the event chain. A List cannot raise an ListChanged event. That is what BindingList exists for.

And as you already tried, when you replace the list in your model by a BindingList it works.

So one solution you have already mentioned, the other would be to use only one shared BindingList for both comboboxes if this is an option for you.

    private void ListDemo()
    {

        var l = new List<string>();
        l.Add("A");

        BindingList<string> blist1 = new BindingList<string>(l);
        BindingList<string> blist2 =new BindingList<string>(l);
        blist1.ListChanged += Blist1_ListChanged;
        blist2.ListChanged += Blist2_ListChanged;


        blist1.Add("B");
        // at this point blist1 and blist2 items count is 2 but only blist1 raised ListChanged
    }

    private void Blist2_ListChanged(object sender, ListChangedEventArgs e)
    {
        //No event fired here when adding B
    }

    private void Blist1_ListChanged(object sender, ListChangedEventArgs e)
    {
       // event fired when adding B
    }

Answer 3

Both ViewModel_Iteration and ViewModel_Create defines their property (VM_Iteration_ListOfStringsInModel and VM_Create_ListOfStringsInModel) by initializing a new object of BindingList which uses the model.simplelistOfStrings as specified source list. So both ViewModels have different object of ListOfStringsInModel --- They don't point to same object.

Definitely you should define the simplelistOfStrings property as BindingList so that a two-way data-binding mechanism can be established in your View and code behind. However, instead of defining a new member variable in both ViewModel_Iteration and ViewModel_Create ViewModels, I would suggest that you should change the property definition as follows :

        public BindingList<string> VM_Iteration_ListOfStringsInModel
        {
            get
            {
                return oModel.simplelistOfStrings;
            }
            set
            {
                oModel.simplelistOfStrings = value;
                this.FirePropertyChanged(nameof(VM_Iteration_ListOfStringsInModel));
            }
        }

And

        public BindingList<string> VM_Create_ListOfStringsInModel
        {
            get
            {

                return oModel.simplelistOfStrings;
            }
            set
            {
                oModel.simplelistOfStrings = value;
                this.FirePropertyChanged(nameof(VM_Create_ListOfStringsInModel));
            }
        }

Another improvement in above approach can be done is that don't use the set at all property definition at all. The Notification is required as reference of property will change when assigned with a new List. So instead of allowing to set a new list, use a approach to Clear and Add new items in the list. That's way the reference of property will remain the same and Two way binding would work without pluming the explicit notification.

ViewModel_Iteration

            public BindingList<string> VM_Iteration_ListOfStringsInModel
            {
                get
                {
                    return oModel.simplelistOfStrings;
                }
            }

ViewModel_Create

            public BindingList<string> VM_Create_ListOfStringsInModel
            {
                get
                {

                    return oModel.simplelistOfStrings;
                }
            }

Usage

VM_Create_ListOfStringsInModel.Clear();
VM_Create_ListOfStringsInModel.Add(item); // Or use AddRange to add Range.