Refactoring Bloated ViewModel

I am writing a PRISM/MVVM/WPF application. It's a LOB application, so there are a lot of complicated rules. I've noticed the View Model is starting to get bloated. There are two main issues.

One is that to maintain MVVM, I'm doing a lot of things that feel hacky like adding a bunch of properties to my VM. The view binds to those properties to keep track of what feels like view specific information. For example, a boolean keeping track of the status of a long running process in the VM, so the view can disable some of its controls while the long running process is working. I've read that this issue could be solved with Attached Behaviors. I'll look more into that. In the example MVVM apps you see online, this isn't a big deal because they are over-simplified.

The other issue is the number of commands in my VM. Right now ther开发者_开发问答e are four commands. I'm defining the commands in the VM using Josh Smith's RelayCommand (basically the DelegateCommand in PRISM) so all the business logic lives in the VM. I considered moving each command into separate unit of works. I'm not sure the best way to do this.

Which patterns are you guys using to keep your VMs clean? I can already feel someone responding with "your view and VM is too complicated, you should break them into many view/VMs". It is certainly not too complicated from a Ux perspective - there are 2 buttons, a combobox, and a listbox. Also, from a logical perspective, it is one cohesive domain. Having said that, I'm very interested in hearing how others are dealing with this type of issue.

Thanks for your input.

I feel your pain. I wrestle with these kinds of questions a lot when working on MVVM apps. One of these days I'll post a laundry list of questions to get input from others like you've done.

I tend to worry very much about "bloat" in my ViewModel base class but not so much in the concrete ViewModel subclasses. It's often tempting to throw a dependency used by 2-3 ViewModels into the base class but it should be avoided.

While I can't presume to know what your idea of bloated is, I can say that I don't think having the "busy" property or commands handled in the VM are bad. One thing you might consider is whether or not the ViewModel can be busy doing more than one thing at once. If so, you might want to go ahead and think about ways to break it up. While I haven't personally seen it in practice, you could probably have your single, cohesive view and a couple ViewModels bound to it.

If your commands are long, or if they can be executed on different targets, I think making the commands self-executing units is a good idea. But it's probably best to be consistent with this approach as to avoid confusing anyone who works on it later. For example if you have a SaveCustomerCommand class that is about 10 lines of code, you probably don't want to use a RelayCommand for everything else.

It'd be nice if there were hard and fast rules for this kinda stuff but I think both the framework and the pattern are still in the evolutionary stage right now.

Without knowing specifics of the ViewModel it's hard to say where the bloat is coming from. It could be non-UI related, like too many lines to query the model. Or it could be UI features that are better implemented all in the UI with triggers or something.

Could you possibly elaborate about your ViewModel?

EDIT Based on comments.

SaveCustomer and DeleteCustomer sound like Model or Service methods, so I'd put them in some sort of Persistence Layer.

Upload/Download Customer Video: Again, these are not ViewModel specific (you might want to do these in another ViewModel) so I'd put them in a Web Service and have the ViewModel command call it.

In general it's worth putting common code (that multiple ViewModels might want to use) into a Service and then throwing that service into your IOC. That way your ViewModels end up being lightweight "directors" of information from the View to either the Model or a Service.

I have two main thoughts on this, but YMMV.

Firstly I would move the implementation specific code from the VM into 'service' which is basically just a class that does the work. This may be for example a 'customerService' class which has the implementation of the save, delete, update methods, and the RelayCommand implementation will call the service to perform the action, and update any view properties as appropriate.

An example would be that the RelayCommand may set the value of a "ShowProgressBar" property, which is bound to the visibility of a progress bar. It would then call the save function in the service, and when complete (this should be asynchronous btw) it should again update the "ShowProgressBar". In this way the ViewModel is controlling the state of the View independently of the core application logic.

Secondly I would look into the base viewmodel you are using and try keep that as simple as possible, only adding components that are common to all your viewmodels. You can then create other base view models or interfaces as appropriate. For example you may decide that all your view models will expose a ShowProgressBar property which can be bound to a progress bar on your page, or you may decide to create a ProgressViewModelBase or IAllowProgress that you inherit/implement in pages that require progress bars.

The View itself should have as little code as possible, you should aim for 0 code-behind (though there are things that can only be done in code-behind unfortunately).

One thing I found to my detriment is that instantiation of complex viewmodels can be expensive in terms of performance, especially if you are registering relaycommands and event aggregator sources/listeners, so keeping the base viewmodel slim us very useful. This is especially important if you are using a ViewModelCollections to display lists of Model objects and where the ViewModel creation occurs on the UI thread (which is likely if the ViewModel is instantiated on the View Creation).