MVVM: Strictness of Views Referencing Models

Question

A coworker and I have differing views on how strictly we should adhere to the MVVM concepts. I try to follow it as closely as possible where he takes shortcuts almost every chance he gets. One of my biggest annoyances is when he has views binding to models. His justification is that a viewmodel would just be a reimplementation of the model but with the added complexity of having to catch and rethrow events (most of our models and viewmodels both implement INotifyPropertyChanged).

For example, in one case, there is TheViewModel which exposes TheModel as a property. TheView's datacontext is set to the TheViewModel. TheView contains a TreeView who's ItemSource is a collection on the TheModel. Each child item in the TreeView is some type of model which contains properties that are directly bound to by the TreeView.

Public Class TheViewModel
  Public Property TheModel as ModelObjectA
End Class

---

Public Class ModelObjectA
  Inherits ModelBase
  Public Property ModelItems as ObservableCollection(Of ModelObjectB)
End Class

---

Public Class ModelObjectB
  Inherits ModelBase
  Public Property Items as ObservableCollection(Of ModelBase)
  Public Property IsValid as Boolean
  Public Property Is as Boolean
End Class

---

Public Class ModelBase
  Public Property Name as String
  Public Property Type as String
End Class

---

<UserControl x:Class="TheView" DataContext="{StaticResource TheViewModel}">
  <TreeView x:Name="ModelTree" ItemsSource="{Binding Path=TheViewModel.ModelItems}" >
    <TreeView.ItemContainerStyle>
      <Style TargetType="TreeViewItem">
        <Style.Triggers>
          <DataTrigger Binding="{Binding IsValid}" Value="False">
            <Setter Property="Background" Value="Red" />
          </DataTrigger>
          <DataTrigger Binding="{Binding IsModified}" Value="True">
            <Setter Property="Foreground" Value="Magenta" />
          </DataTrigger>
        </Style.Triggers>
      </Style>
    </TreeView.ItemContainerStyle>

    <TreeView.Resources>

      <!--ModelObjectB-->
      <HierarchicalDataTemplate DataType="{x:Type mdl:ModelObjectC}" ItemsSource="{Binding Items}">
        <StackPanel>
          <TextBlock Text="{Binding Name}" />
          <TextBlock Text="{Binding Type}" />
        </StackPanel>
      </HierarchicalDataTemplate>

      <!--ModelObjectC-->
      <HierarchicalDataTemplate DataType="{x:Type mdl:ModelObjectC}" ItemsSource="{Binding Items}">
        <StackPanel>
          <TextBlock Text="{Binding Name}" />
          <TextBlock Text="{Binding Type}" />
        </StackPanel>
      </HierarchicalDataTemplate>

      <!--ModelObjectD-->
      <HierarchicalDataTemplate DataType="{x:Type mdl:ModelObjectD}" ItemsSource="{Binding Items}">
        <StackPanel>
          <TextBlock Text="{Binding Name}" />
          <TextBlock Text="{Binding Type}" />
        </StackPanel>
      </HierarchicalDataTemplate>

      <!--Model Base-->
      <DataTemplate DataType="{x:Type mdl:ModelBase}">
        <StackPanel>
          <TextBlock Text="{Binding Name}" />
          <TextBlock Text="{Binding Type}" />
        </StackPanel>
      </DataTemplate>

    </TreeView.Resources>
  </TreeView>

  <ContentPresenter Content="{Binding Path=SelectedItem, ElementName=ModelTree, Converter={StaticResource ModelToViewModelConverter}}" />
</UserControl>

---

Note: This is a simplified version of our real program, and I may have ommited some of the trivial details (such as the UserControl having two child elements and property setter/getters).

I do not believe the above is correct. I think the collections should be exposed to the view as collections of viewmodels (rather than collections of models). Especially when considering that each of the models has a corresponding viewmodel, and in the end, the selected item is transformed into a viewmodel (via a converter).

What does everyone think? Is it ok to have the view binding to models in cases like this, or should the collections be viewmodels? I need some good arguments to either convince my coworker that it should be changed, or to convince myself that the current implementation is the right approach.

Answer 1

You're both right.

In a theoretical perfect world, Views would only know about ViewModels and ViewModels would wrap Models 100%.

The problem is that here in the practical real world, doing that requires writing a bunch of code that:

1. Provides no tangible benefit
2. Consumes time that could be used to do other things
3. Is a potential source of bugs

We have a large MVVM app and I can tell you from experience that keeping ViewModel collections in sync with Model collections is one of more error prone areas of implementing MVVM.

Most of our models support INotifyPropertyChanged and expose ObservableCollections because they can be changed from multiple locations in the application and all of the consumers of the models need to be notified of changes.

At that point, Views can bind to them just fine. Wrapping that stuff with ViewModels just becomes a bunch of tedious plumbing that's a source of bugs. In these situations, it's common for us to expose the model instance as a property on the ViewModel.

Another approach I've used that works quite nicely in certain scenarios is to have the ViewModel inherit from the Model. When you think about it, the ViewModel is quite often simply the Model + some UI logic. By using inheritance instead of composition, you avoid the odd situation of sometimes binding your view to ViewModel.Foo and other times binding to ViewModel.Model.Bar.