2

I'm relatively new to MVVM and I'm trying to understand how INotifyPropertyChanged interface works and how to implement it in my models. The approach that I decided to take was to implement it in each of my Business Object classes.
The problem with that approach is that when I bind my View to a property in a Base class the PropertyChanged event in that base class never gets initialized (is null) and therefore the View does not refresh the data for that element when my Model changes. I was able to reproduce the problem with the example below.

I have a Person Base class:

 public class Person : INotifyPropertyChanged
    {
        #region INotifyProperty

        public event PropertyChangedEventHandler PropertyChanged;

        public void RaisePropertyChanged(string propertyName)
        {
            if (PropertyChanged != null)
            {
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
            }
        }
        #endregion

        public String Name
        {
            get
            {
                return _name;
            }
            set
            {
                _name = value;
                RaisePropertyChanged("Name");
            }
        }
        private String _name;
    }

And I have an Employee class inheriting from my Person Base class:

 public class Employee : Person,INotifyPropertyChanged
    {
        #region INotifyProperty

        public event PropertyChangedEventHandler PropertyChanged;

        public void RaisePropertyChanged(string propertyName)
        {
            if (PropertyChanged != null)
            {
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
            }
        }
        #endregion


        public String EmployeeID
        {
            get
            {
                return _employeeId;
            }
            set
            {
                _employeeId = value;
                RaisePropertyChanged("EmployeeID");
            }
        }
        private String _employeeId;
    }

Here my View Model:

 public class ViewModel : ViewModelBase<ViewModel>
    {
        private Employee _employee;

        public ViewModel()
        {
            ChangeModelCommand = new RelayCommand(param=>this.ChangeModel() , param=>this.CanChangeModel);
            Employee = new Employee()
                         {
                             Name = "BOB",EmployeeID = "1234"
                         };
        }

        public ICommand ChangeModelCommand { get; set; }


        public Employee Employee
        {
            get
            {
                return _employee;
            }
            set
            {
                this._employee = value;
                NotifyPropertyChanged(m=>m.Employee);
            }
        }

        public void ChangeModel()
        {
            MessageBox.Show("CHANGING MODEL");
            this.Employee.Name = "MIKE";
            this.Employee.EmployeeID = "5678";
        }
        public bool CanChangeModel
        {
            get{ return true;}
        }
    }

And finally my View:

<Window.Resources>
    <MVVM_NotificationTest:ViewModel x:Key="Model"></MVVM_NotificationTest:ViewModel>
</Window.Resources>
<Grid DataContext="{StaticResource Model}">
<StackPanel>
    <Label Content="Employee Name"/>
    <TextBox Text="{Binding     Path=Employee.Name,Mode=TwoWay,UpdateSourceTrigger=PropertyChanged}"/>
    <Label Content="Employee ID"/>
    <TextBox Text="{Binding Path=Employee.EmployeeID,Mode=TwoWay,UpdateSourceTrigger=PropertyChanged}"/>
    <Button Content="Change Model" Height="30" Width="100" Margin="5" Command="{Binding Path=ChangeModelCommand}"/>
</StackPanel>
</Grid>

In this example I initialize my Employee VM Property in the VM constructor and then I have a command to modify the EmployeeID (from Employee class) and Name (from Person Class). However, the only UI element in the View that gets updated is the EmployeeID and not the Name (I expected Bob to update to Mike).

While debugging I found that PropertyChanged event was always null in my base class (Person). I also noticed that when I remove the whole #INotifyProperty region from my Employee class everything works fine since it is using the Base Type event and methods.

The problem I have with that is that all my current model classes implement INotifyPropertyChanged explicitly. They all define a PropertyChanged event and implement the RaisePropertyChanged method, which obviously will impact my bindings in my MVVM application.

Lastly, I want to clarify that I do not want wrap my Model properties in my ViewModel and rely on the VM INPC mechanism. I would like to use my Model INPC implementation already in place whithout having to conditionally remove the INPC implementations depending on whether I am inheriting or not from a base type.

In summary, I would like to know what's the best way to implement the INPC in my deeply hierarchical model so that inheritance doesn't break the PropertyEvent propagation as we saw in this example and so my independent classes can be self sufficient as well. Any ideas or suggestions will be greatly appreciated :)

Adolfo Perez
  • 2,834
  • 4
  • 41
  • 61

3 Answers3

5

Simply make RaisePropertyChanged protected and move it into the base class. Currently you will have a lot of duplication that is not necessary.

Something like this:

protected virtual void RaisePropertyChanged(string propertyName);

Many MVVM frameworks provide this for you. For example PRISM has a NotificationObject ViewModel base class.

RichardOD
  • 28,883
  • 9
  • 61
  • 81
  • Precisely what i was thinking :p – Louis Kottmann Jan 29 '12 at 14:11
  • Hi @RichardOD, I just tried that but didn't seem to work. It only works when I move both the PropertyChanged even declaration and the RaisePropertyChanged method to the base class... – Adolfo Perez Jan 29 '12 at 14:18
  • The problem with that is that I have lots of classes generated dynamically, so the INPC implementation was added massively to each and every class. If this is the solution then I would need to have a more complicated logic to eliminate implementation when it is inherited from a base class :( – Adolfo Perez Jan 29 '12 at 14:20
  • 1
    Yes- that's where they both belong. I'd also look at firing events in a Thread safe way too- http://stackoverflow.com/questions/1868877/how-do-i-fire-an-event-safely – RichardOD Jan 29 '12 at 14:21
  • I will mark this as the answer. Now I will have to find a way extend all my dynamically generated classes from a common base class that implements INPC interface. Thanks for your help! – Adolfo Perez Jan 29 '12 at 14:47
  • 1
    Your problem is that you implement INotifyPropertyChanged in your classes, when instead you should just inherit from ModelBase, which already implements this interface and contains a NotifyPropertyChanged method to raise the PropertyChanged event and also marshals the call the UI thread. There is no need for you to implement the interface in your classes - just call NotifyPropertyChanged from each property setter, specifying the property name using a lambda expression. – Anthony Sneed Jan 29 '12 at 20:16
2

You should only implement INPC once, you can use the same raising method in the subclasses.

H.B.
  • 166,899
  • 29
  • 327
  • 400
2

I would also change the raise property changed method to use reflection instead of passing in hard coded strings. I see you did it in your view model but not in your models (where most of the errors tend to occur).

Andrew Barber
  • 39,603
  • 20
  • 94
  • 123
tsells
  • 2,751
  • 1
  • 18
  • 20
  • That is a good observation @tsells. The reason was that I'm using the ViewModelBase from the Simple MVVM Framework from Tony Sneed which implments INPC that way. My model classes will have to implement it in a similar way to avoid typos like you said. – Adolfo Perez Jan 29 '12 at 15:04
  • 1
    To clarify, Simple MVVM Toolkit has a base class that implements INotifyPropertyChanged and has a NotifyPropertyChanged method that accepts a lambda expression for the property name -- not a string -- which makes it type-safe. The method also marshals the call to the UI thread. – Anthony Sneed Jan 29 '12 at 20:12
  • 2
    please also have a look at NotifyPropertyWeaver. Saves you from a lot of pain ;) http://code.google.com/p/notifypropertyweaver/ – doerig Jan 30 '12 at 09:30
  • Thanks Martin, I just looked at the NotifyPropertyWeaver it definitely seems to save you a lot of work. I wonder how it will work with deep class hierarchies(inheritance), I'm not sure if that will result in my same PropertyChanged event disconnection problem upon inheritance, I may give it a try just for curiosity. – Adolfo Perez Jan 31 '12 at 13:01