Self-subscribe to PropertyChanged or addition method call in setter?

Question

Maybe here is already such question, but I didn't find it.

I have MVVM application, and in my ViewModel I have to do some additional actions on changes of some properties (for example, if View changes them). Which approach is better on your mind and why?

1st - Add AdditionalAction call to setter

public class ViewModel: INotifyPropertyChanged
{
  private int _MyProperty;

  public int MyProperty
  {
    get { return _MyProperty; }
    set
    {
      if (_MyProperty == value) return;
      _MyProperty = value;
      RaisePropertyChanged(() => MyProperty);

      // --- ADDITIONAL CODE ---
      AdditionalAction();
    }
  }
}

2nd - Self-subscribe to INotifyPropertyChanged

public class ViewModel: INotifyPropertyChanged
{
  public ViewModel()
  {
    // --- ADDITIONAL CODE ---
    PropertyChanged += OnPropertyChanged;
  }

  private int _MyProperty;

  public int MyProperty
  {
    get { return _MyProperty; }
    set
    {
      if (_MyProperty == value) return;
      _MyProperty = value;
      RaisePropertyChanged(() => MyProperty);
    }
  }

  void PropertyChanged(object sender, PropertyChangedEventArgs e)
  {
    // --- ADDITIONAL CODE ---
    if (e.PropertyName == "MyProperty")
      AdditionalAction();
  }
}

Imagine, that I don't have performance problem or 10'000 objects. It's just View and ViewModel. What is better? First code is "smaller" and has less overhead, but the second (on my mind) is more clear and I can use code snippets for auto-generation properties' code. Even more - in the 2nd case I can write in event handler something like:

On.PropertyChanged(e, p => p.MyProperty, AdditionalAction);

where On is class-helper.

So, what is better on your mind and why?

UPDATED:

OK, it looks like I found yet one approach:

3rd - add "extension point" in RaisePropertyChanged:

public class NotificationObject : INotifyPropertyChanged
{
  void RaisePropertyChanged(Expression<...> property)
  {
    // ... Raise PropertyChanged event
    if (PropertyChanged != null)
      // blah-blah

    // Call extension point
    OnPropertyChanged(property.Name);
  }

  public virtual OnPropertyChanged(string propertyName)
  {
  }
}

public class ViewModel: NotificationObject
{
  private int _MyProperty;

  public int MyProperty
  {
    get { return _MyProperty; }
    set
    {
      if (_MyProperty == value) return;
      _MyProperty = value;
      RaisePropertyChanged(() => MyProperty);
    }
  }

  override OnPropertyChanged(string propertyName)
  {
    if (propertyName == "MyProperty")
      AdditionalAction();
  }
}

This way we don't use event, but all "additional actions" are called from the same "extension point". Is "one place for all addition actions" better than "not transparent workflow"?

Answer 1

I would definitely go for the first method:

• it's clear
• it's explicit in its flow and intention
• it avoids weird (imo) self subscription

The "benefits" of second, which lets you use autogenerated properties is not worth the clearness of the execution flow of the firs case, imo.

Hope this helps.

Answer 2

Here is the "usual" pattern. This allows you to put property-specific code inside the OnX method, and allows derived classes to do the same. No need for a big switch statement, unless of course you're the external listener, but that is par for the course for INotifyPropertyChanged.

public class NotificationObject : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;
    protected void FirePropertyChanged(PropertyChangedEventArgs e)
    {
        PropertyChangedEventHandler handler = PropertyChanged;
        if (handler != null)
            handler(this, e);
    }
}

public class ViewModel : NotificationObject
{
    private int _MyProperty1;
    public int MyProperty1
    {
        get { return _MyProperty1; }
        set
        {
            if (value != _MyProperty1)
            {
                _MyProperty1 = value;
                OnMyProperty1Changed(new PropertyChangedEventArgs("MyProperty1"));
            }
        }
    }

    protected virtual void OnMyProperty1Changed(PropertyChangedEventArgs e)
    {
        FirePropertyChanged(e);
    }

    private int _MyProperty2;
    public int MyProperty2
    {
        get { return _MyProperty2; }
        set
        {
            if (value != _MyProperty2)
            {
                _MyProperty2 = value;
                OnMyProperty2Changed(new PropertyChangedEventArgs("MyProperty2"));
            }
        }
    }

    protected virtual void OnMyProperty2Changed(PropertyChangedEventArgs e)
    {
        FirePropertyChanged(e);
    }
}