Pros and cons of having a WPF specifics in the view model

Question

I'm having trouble deciding what to think about this piece of code:

public SolidColorBrush Brush
{
    get { return IsValid ? _validItemBrush : _invalidItemBrush; }
}

It is part of a view model in my current project and as you can imagine, the Brush will be bound to some text elements in the UI, to indicate (in-)validity of other pieces of data, in an otherwise fairly simple and straightforward dialog.

The proponents of this piece of code say that since we're using WPF, we might as well allow for some simple WPF specific constructs in the view model.

The opponents say that this violates Separation of Concerns, as it clearly dictates style which should be taken care of solely by the view.

Please share your arguments, and if you're not happy with the code above, please share your ideas around alternative solutions. (I'm particularly interested in what you have to say about using DataTemplates).

Is it possible that there is one solution that could be considered best practice?

Answer 1

Personally, I would have the two brushes be defined in XAML, and have the controls that use them switch brushes (in xaml) based on the IsValid property. This could be done very easily with DataTriggers, or even a single IValueConverter - the converter could take 2 brushes and a boolean and swap between them fairly easily.

This keeps the business logic presentation-neutral - a "Brush" is very specific to a specific form of presentation, and a pure View choice. Hard-coding this into the ViewModel violates the single responsibility principle as well as is not a clean separation of concerns.

I would very much keep this in the View, and switch based on the IsValid (bound) property that is ViewModel specific.

Answer 2

While there are circumstances where I might use WPF constructs in the view model, this isn't one of them. Here's why:

• It's harder to change. If you define brushes as resources and use them in styles, changing your application's color scheme can simply be a matter of loading a different resource dictionary. If you hard-code color values in your view models, you have a lot of different things to change if it turns out your end users need different colors.

• It's harder to test. If you want to write a unit test that checks to see if a property is returning the right brush, you have to create a brush in your unit test and compare the values of the two, since it's a reference type.

• In many, maybe even most cases, it doesn't make the code simpler or easier to maintain. You're pretty likely to already be using a style (assuming that you are conversant with styles), since they make just about everything in WPF easier. Binding IsValid to brush colors is just a matter of adding a DataTrigger to a style. Which is where anyone maintaining this code would expect to find it.

There are certainly times when I do use WPF constructs in the view model - for instance, long ago stopped wondering if it was problem if a view model exposed a property of type Visibility. Note that none of the above concerns apply to that case.