21

I have gone through a few MVVM tutorials and I have seen this done both ways. Most use the ViewModel for PropertyChanged (which is what I have been doing), but I came across one that did this in the Model. Are both methods acceptable? If so, what are the benefits/drawbacks of the different methods?

Jason D
  • 2,634
  • 6
  • 33
  • 67
  • 1
    Model is only meant to have the business logic of the application. Better to have the binding Properties in the ViewModel which will decouple business entities and the View – Kurubaran May 31 '13 at 19:42
  • That's what I figured. I was confused when I saw the one example bind in the models. – Jason D May 31 '13 at 19:44
  • 1
    @Coder. The data binding infrastructure already offers more than enough "decoupling". Wrapping all of your entities in pseudo-view models just leads to memory leaks, UI/data inconsistencies, and other subtle bugs. Adding abstraction for abstraction's sake is simply not a good idea. – Jonathan Allen May 31 '13 at 20:51

7 Answers7

39

Microsoft's Patterns and Practices, the inventor of MVVM, and I all disagree with the chosen answer.

Typically, the model implements the facilities that make it easy to bind to the view. This usually means it supports property and collection changed notification through the INotifyPropertyChanged and INotifyCollectionChanged interfaces. Models classes that represent collections of objects typically derive from the ObservableCollection class, which provides an implementation of the INotifyCollectionChanged interface.

-- Microsoft Patterns and Practices: http://msdn.microsoft.com/en-us/library/gg405484%28v=pandp.40%29.aspx#sec4

At this point data binding comes into play. In simple examples, the View is data bound directly to the Model. Parts of the Model are simply displayed in the view by one-way data binding. Other parts of the model can be edited by directly binding controls two-way to the data. For example, a boolean in the Model can be data bound to a CheckBox, or a string field to a TextBox.

-- John Gossman, inventor of MVVM: http://blogs.msdn.com/b/johngossman/archive/2005/10/08/478683.aspx

My own article: http://www.infoq.com/articles/View-Model-Definition

It is an anti-pattern to have a "view-model" that just wraps a model and exposes the same list of properties. The view-model's job is to call external services and expose the individual and collections of models that those services return.

Reasons:

  1. If the model is updated directly, the view-model won't know to fire a property changed event. This causes the UI to go out of sync.
  2. This severely limits your options for sending messages between parent and child view-models.
  3. If the model has its own property changed notification, #1 and 2 aren't a problem. Instead, you have to worry about memory leaks if the wrapper VM goes out of scope but the model doesn't.
  4. If your models are complex, with lots of children objects, then you have to walk the entire tree and create a second object graph that shadows the first one. This can be quite tedious and error prone.
  5. Wrapped collections are particularly difficult to work with. Any time something (UI or backend) inserts or removes an item from a collection, the shadow collection needs to be updated to match. This kind of code is really hard to get right.

That isn't to say you will never need a view-model that wraps a model. If your view-model exposes properties that are significantly different from the model and can't just be papered over with a IValueConverter, then a wrapping view-model makes sense.

Another reason you may need a wrapping view-model is that your data classes don't support data binding for some reason. But even then, it is usually better to just create a normal, bindable model and copy the data from the original data classes.

And of course your view-model is going to have UI specific properties such as which item in a collection is currently selected.

Jonathan Allen
  • 68,373
  • 70
  • 259
  • 447
  • calling external services is part of the infrastructure layer, not part of the viewmodel. Have you ever heard of the repository pattern? Seems you live still in the Forms world. – Mare Infinitus Jun 03 '13 at 17:05
  • perhaps this article is able to shed some light, as it seems very necessary http://msdn.microsoft.com/en-us/magazine/dd419663.aspx#id0090102 – Mare Infinitus Jun 03 '13 at 17:14
  • That article is total garbage and the author has no idea what he is talking about. The only reason he gets away with wrapping each Customer object in a CustomerViewModel is that his Customer object is trivial. If it were a complex object with its own collections (e.g. an Orders property) the whole pattern would collapse. – Jonathan Allen Jun 04 '13 at 06:51
  • 2
    The ViewModel is what makes the call the repository, if you are using one. If you aren't, then it calls the external service directly. The design of the view-model doesn't change either way. – Jonathan Allen Jun 04 '13 at 06:53
  • 2
    The author "gets away" because **he** understands what he is talking about. You propagate that there is no need for a viewmodel at all, which is straight wrong. Binding to a model directly is the exceptional case, not the normal case. And this question here is about "what to do". Perhaps you reread the question and my answer. Thought that some reading could help you understand how MVVM works, but it seems this is lost effort. Especially if the requirements get more complex there is a higher need for a good architecture and design. But why should I bother if you do not care about such things. – Mare Infinitus Jun 04 '13 at 07:15
  • Look at my answer and my other comments. You work with that Josh Smith that wrote the article? And you think he is not that good? So why the heck is he MVP for his work with WPF and you are not? – Mare Infinitus Jun 04 '13 at 07:56
  • Since you seem to be obsessed with personalities over technical details, how about this quote from John Gossman, the man who actually invented the MVVM pattern: "At this point data binding comes into play. In simple examples, the View is data bound directly to the Model. Parts of the Model are simply displayed in the view by one-way data binding. Other parts of the model can be edited by directly binding controls two-way to the data. For example, a boolean in the Model can be data bound to a CheckBox, or a string field to a TextBox." – Jonathan Allen Jun 04 '13 at 08:17
  • 1
    If you had at least read the article, probably even you would find the central parts. **"simple examples"** is actually not production code. Directly from that article: **In practice however, only a small subset of application UI can be data bound directly to the Model** Perhaps you have not that much experience with real world applications, but directly binding to a Model is a simple no-go in the companies and projects that I worked for. But it is a really good start that you open beginners guides to MVVM. I appreciate that. – Mare Infinitus Jun 04 '13 at 09:35
  • 1
    Simple does not mean "not production". Simple means that is the pattern that most of your code is supposed to follow. – Jonathan Allen Jun 04 '13 at 15:51
  • I'm very excited to hear your interpretation of the second emphasized part that I posted. You just make me smile, you are very creative! – Mare Infinitus Jun 04 '13 at 16:43
  • Please have look at this question: http://stackoverflow.com/questions/10437241/mvvm-binding-to-model-while-keeping-model-in-sync-with-a-server-version And I started a new Question on the topic here: http://stackoverflow.com/questions/16947122/binding-to-model-or-viewmodel – Mare Infinitus Jun 05 '13 at 19:10
  • 2
    @JonathanAllen Thank you. You make a few very good points. I agree that wrapper VMs are a waste of code and add nothing to the structure of the project other than to make it more complex. The VMs should be responsible for managing View logic, not delving into the model where the actual business logic resides. Having wrapper VMs on top of that adds nothing but complexity. – Farawin Mar 21 '15 at 10:36
  • @JonathanAllen So, I have a question: what, then, about the Dependency Injection (DI), i.e. the View Model is saturated with references to its model(s) with DI? The VM, then, pulls data from the model (operates on it; translates it; etc), propagates that model to the View, then functions as the mediator between Model and View. Mediation is facilitated by commands, right? I feel like having service(s) that update(s) the model(s) is just adding unnecessary complexity to the program - or, runs the risk of over-modularizing the program (I am super new to all of this). – Thomas Oct 09 '15 at 17:17
  • DI doesn't matter, the MVVM pattern doesn't say how the VM and models are created. MVVM only concerns itself with communication between the three. The VM should never "propagate the model" to the view, it should just expose it and allow data binding to do the rest. As for commands, they are to be used whenever two-way data binding won't work (e.g. button clicks). – Jonathan Allen Oct 10 '15 at 05:00
  • As for services modifying the model, the real MVVM pattern makes that safe (assuming you obey threading rules). But while it gives you the option, whether or not that option makes sense depends on your application. – Jonathan Allen Oct 10 '15 at 05:03
14

The INotifyPropertyChanged (INPC) interface is used for Binding.

So, in the average case, you want to implement it in your ViewModel.

The ViewModel is used to decouple the Model from your View, so there is no need to have INPC in your Model, as you do not want Bindings to your Model.

In most cases, even for smaller properties, you still have a very small ViewModel.

If you want a solid base for MVVM, you are probably going to use some kind of MVVM Framework like caliburn.micro. Using it will give you a ViewModelBase (or here NotifyPropertyChangedBase) so that you do not have to implement those interface members yourself and can just use NotifyOfPropertyChange(() => MyProperty), which is way easier and less error prone.

UPDATE As there seem to be many Windows Forms developers out there, here is an excellent article that will give deeper understanding of what MVVM is about: MSDN Magazine on MVVM

I have linked especially the part about the datamodel, that the question is about.

Mare Infinitus
  • 8,024
  • 8
  • 64
  • 113
  • The tutorial that helped me the most included a framework called MicroMVVM that has been doing very well. It allows for `RaisePropertyChanged(() => Property` and I've had no problems so far. – Jason D May 31 '13 at 19:47
  • It is a good start to use a framework. Caliburn.micro helped me most, as it provides ActionMessages and an EventAggregator, which is really strong. – Mare Infinitus May 31 '13 at 19:50
  • How does Caliburn.micro compare to MVVM Light? I tried that for a bit and didn't care for it much. – Jason D May 31 '13 at 19:52
  • Probably this article will help you http://www.japf.fr/2009/10/a-quick-tour-of-existing-mvvm-frameworks/ – Mare Infinitus May 31 '13 at 19:53
  • Glad if I can help. This comparison seems to be more current: http://catel.catenalogic.com/3.2/index.htm?mvvm_framework_comparison_shee.htm – Mare Infinitus May 31 '13 at 19:58
  • That is a horrible idea. If anything directly changes a property on the model then the event on the view-model won't be raised. This means the UI and underlying data will no longer be in sync. – Jonathan Allen May 31 '13 at 20:45
  • Also, there is absolutely no reason to avoid data binding to models. Data binding was specifically designed so that you could safely listen to property changed notifications on models without introducing memory leaks or other problems. – Jonathan Allen May 31 '13 at 20:48
  • I just tried out Caliburn.Micro and I still much prefer the framework I was using before. – Jason D May 31 '13 at 22:11
  • @JonathanAllen This is separation of concerns. If your model gets changed, your viewmodel already has to know about it. This is part of what the ViewModel is about. – Mare Infinitus Jun 01 '13 at 07:53
  • "Separation of concerns" is not a technical argument, it is what people say when they can't otherwise justify their design. – Jonathan Allen Jun 03 '13 at 16:52
  • @JonathanAllen You just missed a central point of what MVVM is about. Ask a question about it if you do not trust me. – Mare Infinitus Jun 03 '13 at 17:03
  • @DennisE, If you are using C# 5 then you don't need `RaisePropertyChanged(() => Property)` anymore. Instead you can use define the function as `void RaisePropertyChanged([CallerName] string propertyName = null)` and then write `RaisePropertyChanged();` in the setter itself. The compiler will automatically add the property name for you, preventing mistakes. – Jonathan Allen Jun 04 '13 at 18:53
  • 4
    SO asks me to comment as to why I downvoted this answer: because the answer by Jonathan Allen is actually in line with MPP. – Thomas Oct 09 '15 at 17:10
  • 2
    I also disagree with this answer and agree with @JonathanAllen for the same reason. If a view is up (supported by a viewmodel which is looking at a model), and a model changes from somewhere other than the view (perhaps a network command is received that changes the model's property), the view will be out of sync unless the model itself raises property changed. INotifyPropertyChanged is NOT part of WPF (it's in System.ComponentModel) and does NOT know about binding. In reality, WPF binding USES INotifyPropertyChanged to implement its binding mechanism--but they are separate concepts. – Todd Burch Jun 05 '20 at 15:23
5

Would absolutely agree with Jonathan Allen.

If you have nothing to add to your 'View-Model' (Commands, view-specific properties that affect presentation etc.) then I would definitely implement INotifyPropertyChanged in the model and expose that directly (if you can - the 'model' may not be yours). Not only do you end up repeating a lot of boilerplate code, keeping the two in sync is an absolute pain.

INotifyPropertyChanged isn't a view-specific interface, it only does exactly what the name suggests - raises an event when a property changes. WinForms, WPF and Silverlight just happen to support it for Binding - I've certainly used it in for non-presentational purposes!

Charles Mager
  • 25,735
  • 2
  • 35
  • 45
  • You do not have a ViewModel to have a ViewModel. The point is that you can easily add such behavior when you need it, because you made the right design decision in the first place. Implementing a good design when you need it is a big mess, and it cost far more than having a small viewmodel in the first place. The big exception is that you do not have those "anything to add", not having that. The recommend way (by people who have deep knowledge about design, not those from infoq) is to expose the model in your viewmodel then. – Mare Infinitus Jun 04 '13 at 07:36
  • 2
    Ugh no. The whole point of having a View-Model is that it lets you separate the data from the external operations on the data. Once you start mixing ICommands like Save with normal properties like First/Last Name you've left the pattern. Internal operations such as validation and calculated properties (e.g. FullName) should be in the models and unit tested but nothing more. – Jonathan Allen Jun 04 '13 at 07:49
  • 3
    But why would I bother? If I have a View that displays a read-only data grid of 'items', it'd be far simpler to bind that grid to an `ObservableCollection` than to have to slavishly create an `ItemViewModel` to wrap it, code to sync the changes in the underlying items to fire the appropriate `PropertyChanged` events, code to sync the underlying collection to the ViewModel collection etc. It'd be simpler to just bind directly to the model in this case - it's even what the Prism Guidance says: http://msdn.microsoft.com/en-us/library/gg405484%28v=pandp.40%29.aspx#sec4 – Charles Mager Jun 04 '13 at 13:58
  • (Now I re-read what you said, I'm not sure you aren't agreeing with what I just wrote above - but it's rather hard to follow!) – Charles Mager Jun 04 '13 at 14:01
  • 1
    For what it's worth, I agree with what you said @CharlesMager. – Jonathan Allen Jun 04 '13 at 15:49
  • @CharlesMager: Please have look at this question: http://stackoverflow.com/questions/10437241/mvvm-binding-to-model-while-keeping-model-in-sync-with-a-server-version And I started a new Question on the topic here: http://stackoverflow.com/questions/16947122/binding-to-model-or-viewmodel – Mare Infinitus Jun 05 '13 at 19:11
2

The creator of MVVM, JohnGossman, states in this blog article (mentioned by @Jonathan Allen) that:

In simple examples, the View is data bound directly to the Model. Parts of the Model are simply displayed in the view by one-way data binding. Other parts of the model can be edited by directly binding controls two-way to the data. For example, a boolean in the Model can be data bound to a CheckBox, or a string field to a TextBox.

In practice however, only a small subset of application UI can be data bound directly to the Model, especially if the Model is a pre-existing class or data schema over which the application developer has no control.

I prefer to follow practices that are still applicable when the app scales. If "In practice [...], only a small subset of application UI can be data bound directly to the Model", this doesn't seem to be a good practice to follow as I don't plan to tackle only "simple cases" or "a small subset of application UI".
For "simple cases" I wouldn't even be using MVVM to begin with.

Rui Santos
  • 190
  • 2
  • 7
  • Actually, when the app scales up in complexity, the insistence on always creating separate viewmodel reveals its downsides. I’ve done that. Resulted in having to keep in sync two parallel hierarchies of objects. Was a huge headache. OTOH, I don’t know an ideal answer yet. Somehow need to associate view-related properties (e.g. “selected”) with a model, yet be able to guarantee that a collection of vms and a corresponding collection of models keep each other in sync. – ToolmakerSteve May 08 '22 at 18:05
1

As a rule of thumb, any object that you will bind to (even if you don't need Two-Way binding and Property Change Notification), must implement INotifyPropertyChanged. This is because failing to do so May cause memory leaks

Federico Berasategui
  • 43,562
  • 11
  • 100
  • 154
1

INotifyPropertyChanged should be implemented by all the types that are consumed by the view (unless if it only has constant values of course).

Do you return models (not viewmodels) to a view? If yes, then it should implement INotifyPropertyChanged.

lightbricko
  • 2,649
  • 15
  • 21
  • Constant values are tricky. From an API design standpoint you don't want to expose that interface and confuse your consumers. But flaws in the data binding architecture mean that may want to do it anyways. See @HighCore's link for more info. – Jonathan Allen May 31 '13 at 20:49
1

While I'm generally in favor of a model implementing INPC the call for INPC in a composite view model is when it exposes inferred properties that are bindable to the view. IMO since INPC is baked into System.dll, a model implementing it may be considered POCO. For collections there is a performance benefit of model based INPC. On a 64 bit platform a wrapper VM would have an 8 factor multiplier on the byte size (load the SOS debugger extension for actual size) of ObservableCollection<ViewModel> compared to ObservableCollection<Model>.

Rohit
  • 51
  • 3