CODEWITHSUNDEEP
wpfxamlmvvm
Christoffer Lette
Christoffer Lette

Reputation: 14846

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

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?

Upvotes: 7

Views: 492

Answers (3)

Ben Schoepke
Ben Schoepke

Reputation: 789

In cases like yours where it's purely aesthetic I use Triggers or the Visual State Manager to change colors.

Sometimes I do use colors in my ViewModels, but only if its part of my software spec (e.g., the color of the chart displaying a patient's CO2 depends on localization). In that case, I use a Color struct bound property, allowing the View to use the Color for a SolidColorBrush, a GradientStop, or whatever it wants. I initially used a string in #AARRGGBB format to completely remove the WPF dependency but my more seasoned co-workers didn't like that.

Upvotes: 0

Robert Rossney
Robert Rossney

Reputation: 96850

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.

Upvotes: 2

Reed Copsey
Reed Copsey

Reputation: 564751

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.

Upvotes: 8

Related Questions