1

My question is of "What is the best practice?" type, in a very specific case:

I have a View that contains a listbox of checkboxes.

The listbox.ItemsSource is bound to a collection in the ViewModel (List of Person). And each checkbox.IsChecked is bound to Person.IsSelected.

The check/uncheck of a checkbox activates a 'Refresh' method in the ViewModel, that may or may not re-set the Person.IsChecked of all Persons. That, of course, should notify the UI that Person.IsChecked (might have) has changed.

My question is: Who should be responsible of calling ViewMode.Refresh? Should the UI call the method through an event or Command maybe? Or should the ViewModel subscribe to a proper event in Person.

Nir Smadar
  • 357
  • 2
  • 5
  • 15

4 Answers4

3

If the UI is bound to IsChecked, nothing needs to notify it. Or, rather, WPF itself notifies it. That is one of the attractive qualities of MVVM. The view model can simply update its state according to its own set of rules, and the view automatically receives those changes via bindings.

Kent Boogaart
  • 175,602
  • 35
  • 392
  • 393
  • 1
    Ok, but the VM needs to implement INotifyPropertyChaged to make the V notice the value is changed. – qwertoyo Sep 13 '13 at 12:16
1

depending on what you mean by "call"

here are the answers

if you mean create:

  • other ViewModel
  • anything that's not a View or an Model(because non of them should know your VM)

if you talking about executing a Method:

  • self execute (e.g. after PropertyChanged, Command, ...)
  • other ViewModel
  • anything that's not a View or an Model(because non of them should know your VM)

also important in your case:

you say your list is bounded to Collection
and here you could get some trouble because your View changed the Person.IsSelected which should raise a PropertyChanged for your IsSelected.
Now it will stuck because ObservableCollection doesn't get notified if your Item Property Changed that is a known issue. To fix this problem you need to wire up your INotifyPropertyChanged event to the CollectionChanged event from your ObservableCollection. So it can bubble up to your VM and it can execute his Refresh()

Here you can see an example how you could do it.

Community
  • 1
  • 1
WiiMaxx
  • 5,322
  • 8
  • 51
  • 89
1

I think you have to ask yourself, what is the "refresh" in response to?

If it's in response to the check box physically being clicked, then it's the view's responsibility to call it.

If it's in response to the person being selected, the viewmodel should be watching its own collection for changes to IsSelected.

I agree with MehaJain that the latter is harder to set up, but the alternative breaks up the responsibility of a single unit of work between the layers, meaning that the logic will need to be duplicated in any other function that causes IsSelected to be changed. This includes other views binding to the same information, other commands (say you have a button for running a filter that selects certain people), and tests.

Community
  • 1
  • 1
nmclean
  • 7,564
  • 2
  • 28
  • 37
  • Refresh is a response to the person being selected, but there's no SelectedItem attribute here that I can bind to. I don't see any other way than to use an event from Person, which leads to whole bunch of other problems. Do you have a better way? – Nir Smadar Sep 15 '13 at 19:58
  • @NirSmadar I think using the PropertyChanged event of Person is the best way. I don't think it's too problematic -- you basically just need to handle the CollectionChanged for the ObservableCollection of Persons, and subscribe/unsubscribe to them when they are added/removed. – nmclean Sep 16 '13 at 00:00
0

I would prefer to Refresh the View Model through the event or Command.

Because subscribing the collection members is tricky. Let say you want to use the second approach: Subscribing to the Person class changed event. Then it can have following disadvantages:

  • In the constructor of your view model you have to subscribe each "Person" member of your list.
  • In view model whenever collection gets modified then you have to subscribe/unsubscribe the added/deleted members of that list.
  • While disposing your view model, you have to unsubscribe the subscribed changed events.

So, this is an over head if you go for subscribing the change event of Person class.

MehaJain
  • 55
  • 1
  • 10
  • As I see it, calling Refresh when Checked event is raised will cause an endless loop: Refresh is changing the IsSelected prop, so the event will be raised again... Do you have a walk around? – Nir Smadar Sep 15 '13 at 20:09
  • @NirSmadar You could store a boolean in the ViewModel, `isRefreshing`, set to true at the beginning of the Refresh method and false at the end. If `isRefreshing` is already true before you set it, you can break out of the method early to avoid the loop. – nmclean Sep 16 '13 at 00:11