Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value Converters #3

Open
jarnmo opened this issue Dec 12, 2014 · 7 comments
Open

Value Converters #3

jarnmo opened this issue Dec 12, 2014 · 7 comments

Comments

@jarnmo
Copy link
Contributor

jarnmo commented Dec 12, 2014

Feel free to comment and discuss Value Converters here.

What are they good or bad for? Good practices in using them? What kind of issue have you run into? What are the alternatives?

Any thoughts are welcome!

@jarnmo jarnmo changed the title ValueConverters Value Converters Dec 12, 2014
@jarnmo
Copy link
Contributor Author

jarnmo commented Dec 12, 2014

As an attempt to kick off the discussion:

My main problem with Value Converters is that they are way too tedious to create and use. Often it's just easier to create an extra property into your ViewModel that returns the required value in the required format.

As an example case: Your viewmodel has a DateTime Date property, and you want to show the weekday in your view. You either create a new DateTimeToWeekdayConverter class, or add a DateAsWeekday (or similar) get only property to your ViewModel. In latter case you have to remember to NotifyPropertyChanged for the DateAsWeekday whenever the Date changes.

Now, if you put the conversion logic into the ViewModel it's self. You can't reuse it in another view+viewmodel -combo as you can reuse a Value Converter. Therefore, its probably always better to put any conversion logic into an utility class from which it can be reused. But you might still end up basically copy pasting the formatted property into a few viewmodels.

What I've found to be a decent approach in some cases, is to create a container class that holds the 'raw' value and then also offers it in a few bindable formatted properties. For example: You could create a DateTimeViewModel which has a DateTime Value, String Weekday, String Month, int Year, etc properties. In your actual ViewModels that bind to a View, you would then just have this DateTimeViewModel as a property and use the Value property to set the value. Your views could bind to any of the formatted properties and the DateTimeViewModel would handle NotifyPropertyChanges for them. This way you'll avoid some copy pasting, when more than one of your ViewModels have a Date property that needs similar formatting logic.

@dkhmelenko
Copy link
Contributor

To my mind the idea of using ValueConverter is that in ViewModel you don't care about the type, which is required in your View (layout). In your example of Date it's better to use DateTime in ViewModel, so any View could use his own ValueConverter in order to show the date in proper format. But ViewModel will provide the date exactly in DateTime format.

@pcolmer
Copy link

pcolmer commented Feb 13, 2016

I agree that the biggest value from ValueConverters is the avoidance of too many PropertyChanged events firing off. It also generalises the code rather than making it specific to the ViewModel so you end up with cleaner, reusable code as a result.

I don't like the suggestion of a container class because you still end up creating multiple PropertyChanged calls every time the core value changes. Is there a reason why you couldn't have a ValueConverter class that does the same DateTime conversions instead of a container class? That would seem to be more general purpose and project agnostic - simply call the ValueConverter without needed to change all use of DateTime to the container class.

@jarnmo
Copy link
Contributor Author

jarnmo commented Feb 14, 2016

First of all, great to see some discussion kicking off!

I guess, for me, ValueConverters mostly make sense for very general cases, like BooleanToVisibility, InvertBoolean, and StringToUpperCase, or maybe even the DateTimeToDayOfWeek. However, I've seen converters so specific that they couldn't really have usage beyond a specific viewmodel. Like a converter that converts the id of the specific model to an icon. Or what if in this particular view, the datetime should be rendered red when it's less than 3 days from now? In these cases the converter might also contain logic (the id to icon rule, or the three days rule) that should be unit tested with the viewmodel. I guess converters can be unit tested as well, but this rule is tied the particular property of the particular viewmodel.

Additionally, when the converter is so specific that it doesn't have real use beyond a single binding, it's much easier and cleander to just create another property to the viewmodel than create a new ValueConverter, set it as a resource in xaml, and take it into use in a binding.

@pcolmer, do you have some experience on the performance impact of PropertyChanged calls? I wouldn't expect the call itself to cause much of a performance hit. The UI update following it might always be heavy though, but I don't think ValueConverters would help there. Actually, the wrapper class (or an extra viewmodel property) could avoid notifying property change if the 'evaluated' value doesn't change even if the 'base' value does. For example if date changes from today seven days onwards, the datetime changes, but the DayOfWeek wouldn't. The viewmodel could avoid firing PropertyChanged for the DayOfWeek property and bindings to that property would stay idle. With a binding using a DateTimeToDayOfWeek converter and binding to the Date property, the binding would fire, and the converter would have to be run. Then it would be up to the UI Control to not update itself to the new (the same) value.

Btw, I just realized that DateTimeOffset actually has a DayOfWeek property, but for the sake of the example, let's just ignore that.

@pcolmer
Copy link

pcolmer commented Feb 14, 2016

No, I don't have any experience on the performance; it just seems to me that having written code that hooks onto PropertyChanged events as a means of bubbling events from one bit of code to another, I do try to minimise unnecessary events firing.

It also occurred to me, and I forgot to write this the first time, that there is complexity around needing to remember to add additional PropertyChanged calls when the underlying value changes. As earlier noted, you have to remember to trigger PropertyChanged calls for all of the related "converted" getter properties. That can lead to head-scratching when the UI doesn't update as expected.

I take the point, though, about specialist conversions. I think, though, that even if they were used in a single ViewModel, wouldn't that make the XAML more readable and, again, avoid the need for additional code to ensure that when the value changes, the associated conversion property is marked as changed as well?

I've been giving quite a lot of thought as to the readability of my code as I prepare to move my app to Windows 10. I'm thinking that the UI is going to be virtually brand-new and, as such, I'm keen to make the XAML as clean as possible and, for me, converters seem to be a neat way of abstracting an underlying value to the visual representation and isn't that what MVVM is partly about?

@dkhmelenko
Copy link
Contributor

I'm thinking that the UI is going to be virtually brand-new and, as such, I'm keen to make the XAML as clean as possible and, for me, converters seem to be a neat way of abstracting an underlying value to the visual representation and isn't that what MVVM is partly about?

👍

@jarnmo
Copy link
Contributor Author

jarnmo commented Feb 14, 2016

Fair enough, it's definitely not a bad idea to avoid firing unnecessary events. Also, It's definitely true that there is a bit of complexity and certainly an opportunity for error in firing PropertyChanged events for the 'converted' properties. C# 6 nameof-operator and reactive programming (with the help of ReactiveProperty or similar) can greatly help with this, but don't necessary make it a non-issue. However, you can also forget to add the value converter to the binding and while that kind of an error would be more easy to notice, it can't be unit tested.

I guess readability is largely up to personal preference, but I do find the first option in the following examples more readable:

<! -- 1: DataContext of 'Event' -->
<TextBlock Text="{Binding DayOfWeek}"/>
<!-- or -->
<TextBlock Text="{Binding Date, Converter={StaticResource DateTimeToDayOfWeek}}"/>

<! -- 2: DataContext of 'Person' -->
<TextBlock Text="{Binding BirthdayAsDayOfWeek}"/>
<!-- or -->
<TextBlock Text="{Binding Birthday, Converter={StaticResource DateTimeToDayOfWeek}}"/>

<! -- 3: DataContext of 'MenuItem' -->
<Image Source="{Binding Icon}"/>
<!-- or -->
<Image Source="{Binding Id, Converter={StaticResource MenuItemIdToIcon}}"/>

Obviously how it reads in XAML also depends on how you name your derived properties and converter resources. And while you might save a bit in XAML, you'll end up with a bit more code in your viewmodel. I don't tend to try to minimize the number of lines, but I do try to maximize the readability.

Also, another factor here is converter parameters. They definitely make the XAML even less readable, but on the other hand make value converters more flexible. My feeling is though, that whatever the use of the parameter, a viewmodel should be able to easily do the same thing for a derived property.

I did just recall that there is at least one usecase for value converters that a derived property just wouldn't work for, and that is for values that never touch the viewmodel. I guess this would mostly mean UI elements binding to values of other UI elements. For example for some layout, state, or animation handling. In such case, even a very specific ValueConverter probably makes more sense than trying to route the values through a viewmodel. On the other hand, I guess code behinds could be used in such cases as well. Anyway, I've written generic ValueConverters e.g. for numeric operations to use in such cases (oh how I wish I could do them in XAML without a ValueConverter).

And yes, as you bring up, there is also the question of where does certain logic (conversion) belong to in a MVVM architecture. I would place ValueConverters into the View layer, and when they are used to modify bound values between UI elements, there's no question that in the View layer they are. However, if a ValueConverter is used, for example, to hide/show an action based on a UserRights property, it does seem to get to the viewmodel territory. I guess even the DateTimeToDayOfWeek converter would step a bit into the viewmodel territory. It's not strictly a 'formatting' conversion. There are actually logic and rules behind what is the day of week for a given date, so it's not strictly a view thing.

Now, obviously it would be awesome to come up with a 'best practice' when to use ValueConverters (and when not to). So far at least a few usecases seem quite clear to me:

  1. Simple type conversions to make the binding work at all. For example a converter that converts a Boolean to Visibility.
  2. Conversions or adjustments to purely view related values that don't touch the viewmodel. For example a converter that converts width of the view to a visual state.
  3. Purely view dictated formatting conversions. For example a converter that converts a string to upper case characters.

However, I would like to be able to 'rule' some of the technically possible usages out as well. And be able to offer better solutions for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants