Are value converters more trouble than they're worth?

Question

I'm working on a WPF application with views that require numerous value conversions. Initially, my philosophy (inspired in part by this lively debate on XAML Disciples) was that I should make the view model strictly about supporting the data requirements of the view. This meant that any value conversions required to turn data into things like visibilities, brushes, sizes, etc. would be handled with value converters and multi-value converters. Conceptually, this seemed quite elegant. The view model and view would both have a distinct purpose and be nicely decoupled. A clear line would be drawn between "data" and "look".

Well, after giving this strategy "the old college try", I'm having some doubts whether I want to continue developing this way. I'm actually strongly considering dumping the value converters and placing the responsibility for (almost) all value conversion squarely in the hands of the view model.

The reality of using value converters just doesn't seem to be measuring up to the apparent value of cleanly separated concerns. My biggest issue with value converters is that they are tedious to use. You have to create a new class, implement IValueConverter or IMultiValueConverter, cast the value or values from object to the correct type, test for DependencyProperty.Unset (at least for multi-value converters), write the conversion logic, register the converter in a resource dictionary [see update below], and finally, hook up the converter using rather verbose XAML (which requires use of magic strings for both the binding(s) and the name of the converter [see update below]). The debugging process is no picnic either, as error messages are often cryptic, especially in Visual Studio's design mode/Expression Blend.

This isn't to say that the alternative--making the view model responsible for all value conversion--is an improvement. This could very well be a matter of the grass being greener on the other side. Besides losing the elegant separation of concerns, you have to write a bunch of derived properties and make sure you conscientiously call RaisePropertyChanged(() => DerivedProperty) when setting base properties, which could prove to be an unpleasant maintenance issue.

The following is an initial list I put together of the pros and cons of allowing view models to handle conversion logic and doing away with value converters:

• Pros:
  • Fewer total bindings since multi-converters are eliminated
  • Fewer magic strings (binding paths + converter resource names)
  • No more registering each converter (plus maintaining this list)
  • Less work to write each converter (no implementing interfaces or casting required)
  • Can easily inject dependencies to help with conversions (e.g., color tables)
  • XAML markup is less verbose and easier to read
  • Converter reuse still possible (although some planning is required)
  • No mysterious issues with DependencyProperty.Unset (a problem I noticed with multi-value converters)

*Strikethroughs indicate benefits that disappear if you use markup extensions (see update below)

• Cons:
  • Stronger coupling between view model and view (e.g., properties must deal with concepts like visibility and brushes)
  • More total properties to allow direct mapping for every binding in view
  • RaisePropertyChanged must be called for each derived property (see Update 2 below)
  • Must still rely on converters if the conversion is based on a property of a UI element

So, as you can probably tell, I have some heartburn about this issue. I'm very hesitant to go down the road of refactoring only to realize that the coding process is just as inefficient and tedious whether I use value converters or expose numerous value conversion properties in my view model.

Am I missing any pros/cons? For those who have tried both means of value conversion, which did you find worked better for you and why? Are there any other alternatives? (The disciples mentioned something about type descriptor providers, but I couldn't get a handle on what they were talking about. Any insight on this would be appreciated.)

---

Update

I found out today that it's possible to use something called a "markup extension" to eliminate the need to register value converters. In fact, it not only eliminates the need to register them, but it actually provides intellisense for selecting a converter when you type Converter=. Here is the article that got me started: http://www.wpftutorial.net/ValueConverters.html.

The ability to use a markup extension changes the balance somewhat in my pros and cons listing and discussion above (see strikethroughs).

As a result of this revelation, I'm experimenting with a hybrid system where I use converters for BoolToVisibility and what I call MatchToVisibility and the view model for all other conversions. MatchToVisibility is basically a converter that lets me check if the bound value (usually an enum) matches one or more values specified in XAML.

Example:

Visibility="{Binding Status, Converter={vc:MatchToVisibility
            IfTrue=Visible, IfFalse=Hidden, Value1=Finished, Value2=Canceled}}"

Basically what this does is check if the status is either Finished or Canceled. If it is, then the visibility gets sets to "Visible". Otherwise, it gets sets to "Hidden". This turned out to be a very common scenario, and having this converter saved me about 15 properties on my view model (plus associated RaisePropertyChanged statements). Note that when you type Converter={vc:, "MatchToVisibility" shows up in an intellisense menu. This noticeably reduces the chance of errors and makes using value converters less tedious (you don't have to remember or look up the name of the value converter you want).

In case you're curious, I'll paste the code below. One important feature of this implementation of MatchToVisibility is that it checks to see if the bound value is an enum, and if it is, it checks to make sure Value1, Value2, etc. are also enums of the same type. This provides a design-time and run-time check of whether any of the enum values are mistyped. To improve this to a compile-time check, you can use the following instead (I typed this by hand so please forgive me if I made any mistakes):

Visibility="{Binding Status, Converter={vc:MatchToVisibility
            IfTrue={x:Type {win:Visibility.Visible}},
            IfFalse={x:Type {win:Visibility.Hidden}},
            Value1={x:Type {enum:Status.Finished}},
            Value2={x:Type {enum:Status.Canceled}}"

While this is safer, it's just too verbose to be worth it for me. I might as well just use a property on the view model if I'm going to do this. Anyway, I'm finding that the design-time check is perfectly adequate for the scenarios I've tried so far.

Here's the code for MatchToVisibility

[ValueConversion(typeof(object), typeof(Visibility))]
public class MatchToVisibility : BaseValueConverter
{
    [ConstructorArgument("ifTrue")]
    public object IfTrue { get; set; }

    [ConstructorArgument("ifFalse")]
    public object IfFalse { get; set; }

    [ConstructorArgument("value1")]
    public object Value1 { get; set; }

    [ConstructorArgument("value2")]
    public object Value2 { get; set; }

    [ConstructorArgument("value3")]
    public object Value3 { get; set; }

    [ConstructorArgument("value4")]
    public object Value4 { get; set; }

    [ConstructorArgument("value5")]
    public object Value5 { get; set; }

    public MatchToVisibility() { }

    public MatchToVisibility(
        object ifTrue, object ifFalse,
        object value1, object value2 = null, object value3 = null,
        object value4 = null, object value5 = null)
    {
        IfTrue = ifTrue;
        IfFalse = ifFalse;
        Value1 = value1;
        Value2 = value2;
        Value3 = value3;
        Value4 = value4;
        Value5 = value5;
    }

    public override object Convert(
        object value, Type targetType, object parameter, CultureInfo culture)
    {
        var ifTrue = IfTrue.ToString().ToEnum<Visibility>();
        var ifFalse = IfFalse.ToString().ToEnum<Visibility>();
        var values = new[] { Value1, Value2, Value3, Value4, Value5 };
        var valueStrings = values.Cast<string>();
        bool isMatch;
        if (Enum.IsDefined(value.GetType(), value))
        {
            var valueEnums = valueStrings.Select(vs => vs == null ? null : Enum.Parse(value.GetType(), vs));
            isMatch = valueEnums.ToList().Contains(value);
        }
        else
            isMatch = valueStrings.Contains(value.ToString());
        return isMatch ? ifTrue : ifFalse;
    }
}

Here's the code for BaseValueConverter

// this is how the markup extension capability gets wired up
public abstract class BaseValueConverter : MarkupExtension, IValueConverter
{
    public override object ProvideValue(IServiceProvider serviceProvider)
    {
        return this;
    }

    public abstract object Convert(
        object value, Type targetType, object parameter, CultureInfo culture);

    public virtual object ConvertBack(
        object value, Type targetType, object parameter, CultureInfo culture)
    {
        throw new NotImplementedException();
    }
}

Here's the ToEnum extension method

public static TEnum ToEnum<TEnum>(this string text)
{
    return (TEnum)Enum.Parse(typeof(TEnum), text);
}

---

Update 2

Since I posted this question, I've come across an open-source project that uses "IL weaving" to inject NotifyPropertyChanged code for properties and dependent properties. This makes implementing Josh Smith's vision of the view model as a "value converter on steroids" an absolute breeze. You can just use "Auto-Implemented Properties", and the weaver will do the rest.

Example:

If I enter this code:

public string GivenName { get; set; }
public string FamilyName { get; set; }

public string FullName
{
    get
    {
        return string.Format("{0} {1}", GivenName, FamilyName);
    }
}

...this is what gets compiled:

string givenNames;
public string GivenNames
{
    get { return givenName; }
    set
    {
        if (value != givenName)
        {
            givenNames = value;
            OnPropertyChanged("GivenName");
            OnPropertyChanged("FullName");
        }
    }
}

string familyName;
public string FamilyName
{
    get { return familyName; }
    set 
    {
        if (value != familyName)
        {
            familyName = value;
            OnPropertyChanged("FamilyName");
            OnPropertyChanged("FullName");
        }
    }
}

public string FullName
{
    get
    {
        return string.Format("{0} {1}", GivenName, FamilyName);
    }
}

That's a huge savings in the amount of code you have to type, read, scroll past, etc. More importantly, though, it saves you from having to figure out what your dependencies are. You can add new "property gets" like FullName without having to painstakingly go up the chain of dependencies to add in RaisePropertyChanged() calls.

What is this open-source project called? The original version is called "NotifyPropertyWeaver", but the owner (Simon Potter) has since created a platform called "Fody" for hosting a whole series of IL weavers. The equivalent of NotifyPropertyWeaver under this new platform is called PropertyChanged.Fody.

• Fody setup instructions: http://code.google.com/p/fody/wiki/SampleUsage (replace "Virtuosity" with "PropertyChanged")
• PropertyChanged.Fody project site: http://code.google.com/p/propertychanged/

If you'd prefer to go with NotifyPropertyWeaver (which a little simpler to install, but won't necessarily be updated in the future beyond bug fixes), here is the project site: http://code.google.com/p/notifypropertyweaver/

Either way, these IL weaver solutions completely change the calculus in the debate between view model on steroids vs. value converters.

Answer 1

I have used ValueConverters in some cases and put the logic in the ViewModel in others. My feeling is that a ValueConverter becomes part of the View layer, so if the logic is really part of the View then put it there, otherwise put it in the ViewModel.

Personally I don't see a problem with a ViewModel dealing with View-specific concepts like Brushes because in my applications a ViewModel only exists as a testable and bindable surface for the View. However, some people put a lot of business logic in the ViewModel (I do not) and in that case the ViewModel is more like a part of their business layer, so in that case I wouldn't want WPF-specific stuff in there.

I prefer a different separation:

• View - WPF stuff, sometimes untestable (like XAML and code-behind) but also ValueConverters
• ViewModel - testable and bindable class that is also WPF-specific
• EditModel - part of the business layer that represents my model during manipulation
• EntityModel - part of the business layer that represents my model as persisted
• Repository - responsible for persistence of the EntityModel to the database

So, the way I do it, I have little use for ValueConverters

The way I got away from some of your "Con's" is to make my ViewModel's very generic. For instance, one ViewModel I have, called ChangeValueViewModel implements a Label property and a Value property. On the View there's a Label that binds to the Label property and a TextBox that binds to the Value property.

I then have a ChangeValueView which is a DataTemplate keyed off of the ChangeValueViewModel type. Whenever WPF sees that ViewModel it applies that View. The constructor of my ChangeValueViewModel takes the interaction logic it needs to refresh its state from the EditModel (usually just passing in a Func<string>) and the action it needs to take when the user edits the Value (just an Action that executes some logic in the EditModel).

The parent ViewModel (for the screen) takes an EditModel in its constructor and just instantiates the appropriate elementary ViewModels such as ChangeValueViewModel. Since the parent ViewModel is injecting the action to take when the user makes any change, it can intercept all of these actions and take other actions. Therefore, the injected edit action for a ChangeValueViewModel might look like:

(string newValue) =>
{
    editModel.SomeField = newValue;
    foreach(var childViewModel in this.childViewModels)
    {
        childViewModel.RefreshStateFromEditModel();
    }
}

Obviously the foreach loop can be refactored elsewhere, but what this does is take the action, apply it to the model, then (assuming the model has updated its state in some unknown way), tells all the child ViewModels to go and get their state from the model again. If the state has changed, they are responsible for executing their PropertyChanged events, as necessary.

That handles the interaction between, say, a list box and a details panel quite nicely. When the user selects a new choice, it updates the EditModel with the choice, and the EditModel changes the values of the properties exposed for the detail panel. The ViewModel children that are responsible for displaying the detail panel information automatically get notified that they need to check for new values, and if they've changed, they fire their PropertyChanged events.

Answer 2

If the Conversion is something View-Related, such as deciding the visibility of an object, determining which image to display, or figuring out what brush color to use, I always put my converters in the View.

If its Business-Related, such as determining if a field should be masked, or if a user has permission to perform an action, then the conversion happens in my ViewModel.

From your examples, I think you're missing a big piece of WPF: DataTriggers. You seem to be using converters for determining conditional values, but converters should really be for converting one data type into another.

In your example above

Example: say you had a stereo system that could either be in radio, CD, or MP3 mode. Assume there are visuals corresponding to each mode in different parts of the UI. You could either (1) let the view decide which graphics correspond to which mode and turn them on/off accordingly, (2) expose properties on the view model for each mode value (e.g., IsModeRadio, IsModeCD), or (3) expose properties on the view model for each graphical element/group (e.g., IsRadioLightOn, IsCDButtonGroupOn). (1) seemed a natural fit for my view, because it already has mode awareness. What do you think in this case?

I would use a DataTrigger to determine which image to display, not a Converter. A converter is for converting one data type to another, while a trigger is used to determine some properties based on a value.

<Style x:Key="RadioImageStyle">
    <Setter Property="Source" Value="{StaticResource RadioImage}" />
    <Style.Triggers>
        <DataTrigger Binding="{Binding Mode}" Value="CD">
            <Setter Property="Source" Value="{StaticResource CDImage}" />
        </DataTrigger>
        <DataTrigger Binding="{Binding Mode}" Value="MP3">
            <Setter Property="Source" Value="{StaticResource MP3Image}" />
        </DataTrigger>
    </Style.Triggers>
</Style>

The only time I would consider using a Converter for this is if the bound value actually contained the image data, and I needed to convert it into a data type that the UI could understand. For example, if the data source contained a property called ImageFilePath, than I would consider using a Converter to convert the string containing the image file location to a BitmapImage which could be used as the Source for my Image

<Style x:Key="RadioImageStyle">
    <Setter Property="Source" Value="{Binding ImageFilePath, 
            Converter={StaticResource StringPathToBitmapConverter}}" />
</Style>

The end result is I have one library namespace full of generic converters which convert one data type to another, and I rarely have to code a new converter. There are occasions when I will want converters for specific conversions, but they are infrequent enough that I never mind writing them.

Answer 3

It depends on what you're testing, if anything.

No tests: intermix View code w/ ViewModel at will (you can always refactor later).
Tests on ViewModel and/or lower: use converters.
Tests on Model layers and/or lower: intermix View code w/ ViewModel at will

ViewModel abstracts the Model for the View. Personally, I'd use ViewModel for Brushes, etc and skip the converters. Test on the layer(s) where data is in its "purest" form (ie. Model layers).

Answer 4

This probably won't solve all the issues you mentioned, but there are two points to consider:

First, you need to place the converter code somewhere in your first strategy. Do you consider that part of the view or the view model? If it's part of the view, why not place the view-specific properties in the view instead of the view-model?

Second, it sounds like your non-converter design attempts to modify the actual objects properties that already exist. It sounds like they already implement INotifyPropertyChanged, so why not use create a view-specific wrapper object to bind to? Here's a simple example:

public class RealData
{
    private bool mIsInteresting;
    public bool IsInteresting
    {
        get { return mIsInteresting; }
        set 
        {
            if (mIsInteresting != null) 
            {
                mIsInteresting = value;
                RaisePropertyChanged("IsInteresting");
            }
        }
    }
}

public class RealDataView
{
    private RealData mRealData;

    public RealDataView(RealData data)
    {
        mRealData = data;
        mRealData.PropertyChanged += OnRealDataPropertyChanged;
    }

    public Visibility IsVisiblyInteresting
    {
       get { return mRealData.IsInteresting ? Visibility.Visible : Visibility.Hidden; }
       set { mRealData.IsInteresting = (value == Visibility.Visible); }
    }

    private void OnRealDataPropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "IsInteresting") 
        {
            RaisePropertyChanged(this, "IsVisiblyInteresting");
        }
    }
}

Answer 5

Sometimes it is good to use a value converter to take advantage of virtualization.

An example of this what in a project where we had to display bitmasked data for hundreds of thousands of cells in a grid. When we decoded the bitmasks in the view model for every single cell the program took way too long to load.

But when we created a value converter that decoded a single cell the program loaded in a fraction of the time and was just as responsive because the converter is only called when the user is looking at a particular cell (and it would only need to be called a max of thirty times any time the user shifted their view on the grid).

I don't know how MVVM complaint that solution was, but it cut the load time by 95%.