How do you handle "SelectedItemChanged" events in MVMM ViewModel?

Question

I have some logic that depends on whether two properties are set, since it runs when both properties have a value. For instance:

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

This code will need to be executed each time Property1 or Property2 changes, but it’s hard for me to figure out how to do this in a stylistically acceptable way. Here is the choice I see:

1) Method call from ViewModel

I have no problem with this conceptually, since the logic is still in the ViewModel - I am not "without code" Nazis. However, the trigger logic (when property changes) is still in the user interface layer, which I don't like. Codebehind will look like this:

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

2) Method call from Property Setter

This approach seems the most “clean”, but it also seems ugly, as if logic is hidden. It will look like this:

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

3) Connect to the PropertyChanged event

Now I think that this may be the right approach, but it is strange to connect to an event modified by properties in an implementing viewmodel. It will 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 looking at some source code with this requirement, which approach would you rather see implemented (and why?). Thanks for any input.

Answer 1

I don’t think that doing it in the setter is ugly ... in fact, this is probably the best of the three approaches that you mentioned, because when you read the code, you immediately see that changing the value of Property1 or Property2 will recalculate Property3 ; this is not obvious at all with two other approaches.

However, I would not use any of these options. I think the best way to do this is to make only Property3 read-only and compute its value in the getter according to Property1 and Property2 :

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

So in the settings for Property1 and Property2 you just need to call NotifyPropertyChanged for Property3 .

Answer 2

(2) is how I usually do it.

However, I wondered if there could 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)); } } } 

Answer 3

Yes @ Thomas is right. Approach 2 is the ideal way to migrate to a WPF environment. If you added a ListBox.SelectedValue TwoWay binding to Property1

Your (1) is invalid because it displays business logic in a view. (3) is an unnecessary event handling, which in any case is triggered by the code Set1 Property1. So it's better to call it straight from the setter. So the MVVM path is (2)