Refactoring Bloated ViewModel

Question

Hi,

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 there 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.

Answer 1

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.

Answer 2

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.