How do you handle 'SelectedItemChanged' events in a MVVM ViewModel?

Question

I have some logic that depends upon two properties being set, as it executes when both properties have a value. For example:

private void DoCalc() {
  if (string.IsNullOrEmpty(Property1) || string.IsNullOrEmpty(Property2))
    return;
  Property3 = Property1 + " " + Property2;
}

That code would need to be executed every time Property1 or Property2 changed, but I'm having trouble figuring out how to do it in a stylistically acceptable manner. Here are the choices as I see them:

1) Call method from ViewModel

I don't have a problem with this conceptually, as the logic is still in the ViewModel - I'm not a 'No code-behind' nazi. However, the 'trigger' logic (when either property changes) is still in the UI layer, which I don't love. The codebehind would look like this:

void ComboBox_Property1_SelectedItemChanged(object sender, RoutedEventArgs e) {
  viewModel.DoCalc();
}

2) Call method from Property Setter

This approach seems the most 'pure', but it also seems ugly, as if the logic is hidden. It would look like this:

public string Property1 {
  get {return property1;}
  set {
    if (property1 != value) {
      property1 = value;
      NotifyPropertyChanged("Property1");
      DoCalc();
    }
  }
}

3) Hook into the PropertyChanged event

I'm now thinking this might be the right approach, but it feels weird to hook into the property changed event in the implementing viewmodel. It would look something like this:

public ViewModel() {
  this.PropertyChanged += new PropertyChangedEventHandler(ViewModel_PropertyChanged);
}

void ViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e) {
  if (e.PropertyName == "Property1" || e.PropertyName == "Property2") {
    DoCalc();
  }
}

So, my question is, if you were browsing through some source code with that requirement, which approach would you prefer to see implemented (and why?). Thanks for any input.

Answer 1

I don't think doing it in the setter is ugly... actually it's probably the best of the 3 approaches you mentioned, because when you read the code, you immediately see that changing the value of Property1 or Property2 will recompute Property3 ; this is not obvious at all with the 2 other approaches.

However, I would use neither of these options. I think a better way of doing it would be to make Property3 read only, and compute its value in the getter, according to Property1 and Property2 :

public string Property3
{
    get { return Property3 = Property1 + " " + Property2; }
}

That way, in the setters of Property1 and Property2, you just need to call NotifyPropertyChanged for Property3 as well.

Answer 2

Yes @Thomas is right. Approach 2 is the perfect way to go in a WPF environment. Provided you have added ListBox.SelectedValue TwoWay binding to the Property1

Your (1) is invalid because that exposes the business logic on to the View. (3) is an unnecessary Event handling, which is anyway triggered by the Property1 setter code. So better to call it directly from the Setter. So MVVM way is (2)

Answer 3

(2) is the way I usually do it.

That said, it got me wondering if there might be another way to do this type of thing using the Rx framework: http://msdn.microsoft.com/en-us/devlabs/ee794896.aspx

So this is what I came up with (CAVEAT - Don't do it this way!) ;)

public class ViewModel : INotifyPropertyChanged
{
    public ViewModel()
    {
        var o1 =
            Observable.FromEvent<PropertyChangedEventArgs>(this, "PropertyChanged");
        o1.Subscribe(e => Debug.WriteLine(e.EventArgs.PropertyName));
        var o2 = o1.SkipWhile(e => e.EventArgs.PropertyName != "Property1");
        var o3 = o1.SkipWhile(e => e.EventArgs.PropertyName != "Property2");
        var o4 = o1.SkipWhile(e => e.EventArgs.PropertyName != "Result");
        var o5 = Observable.CombineLatest(o2, o3, (e1, e2) => DoStuff()).TakeUntil(o4);
        o5.Subscribe(o => Debug.WriteLine("Got Prop1 and Prop2"));
    }

    public string DoStuff()
    {
        return Result = string.Concat(Property1, Property2);                
    }

    private string _property1;
    public string Property1
    {
        get { return _property1; }
        set
        {
            _property1 = value;
            OnNotifyPropertyChanged("Property1");
        }
    }
    private string _property2;
    public string Property2
    {
        get { return _property2; }
        set
        {
            _property2 = value;
            OnNotifyPropertyChanged("Property2");
        }
    }
    private string _result;
    public string Result
    {
        get { return _result; }
        set
        {
            _result = value;
            OnNotifyPropertyChanged("Result");
        }
    }
    public event PropertyChangedEventHandler PropertyChanged;
    private void OnNotifyPropertyChanged(string name)
    {
        var handler = PropertyChanged;
        if(handler != null)
        {
            handler(this, new PropertyChangedEventArgs(name));
        }
    }
}