Using enum descriptions to string/text binding

Question

In my WP8 application I need to show a couple of radio buttons to let the user select a value out of an enum. I don't want to hardcode either the value or the text (description) showed to the user. I would like my Model/ViewModel drive them for me.

As of now, this is what I have:

//enum defined in model
 public enum Units
    {
        [Description ("Meters/km")]
        Metric,
        [Description ("Feet/Miles")]
        Imperial
    }

Converters defined to handle string and Boolean conversions

//<Summary>Converts ENum to boolean and back
// Convert: uses parameter passed in, returns true if current value of the Enum matches parameter
//ConvertBack: if value is true, sets the value of the ENum to parameter passed in
//</summary>
[ValueConversion (typeof (Enum), typeof (Boolean))]
public class EnumToBooleanConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        string parameterString = parameter as string;
        if ( parameterString == null )
            return DependencyProperty.UnsetValue;

        if ( Enum.IsDefined (value.GetType (), value) == false )
            return DependencyProperty.UnsetValue;

        object parameterValue = Enum.Parse (value.GetType (), parameterString);

        return parameterValue.Equals (value);

    }

    public object ConvertBack(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        return value.Equals (true) ? Enum.Parse(targetType, parameter as String) : DependencyProperty.UnsetValue;
    }
}

//<Summary>Converts ENum to strings, uses Description for the parameter passed in, or parameter as string</summary>
[ValueConversion (typeof (Enum), typeof (String))]
public class EnumToStringConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        string parameterString = parameter as string;
        if ( parameterString == null )
            return DependencyProperty.UnsetValue;

        if ( Enum.IsDefined (value.GetType (), value) == false )
            return DependencyProperty.UnsetValue;

        var desc = (value.GetType ().GetField (parameterString).GetCustomAttributes (typeof (DescriptionAttribute), false).FirstOrDefault() as DescriptionAttribute);
        if ( desc != null )
            return desc.Description;
        else
            return parameter.ToString ();
    }

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

In the XAML:

   <Grid  Margin="0,10,0,0">
    <Grid.RowDefinitions>
        <RowDefinition />
        <RowDefinition />
    </Grid.RowDefinitions>
    <Grid.ColumnDefinitions>
        <ColumnDefinition Width="Auto" />
        <ColumnDefinition Width="Auto" />
    </Grid.ColumnDefinitions>
    <TextBlock Text="How do you measure distance?"
               Margin="6,0,0,0"
               Grid.ColumnSpan="2"
               Grid.Row="0" />
    <RadioButton Grid.Row="1"
                 Grid.Column="0"
                 Content="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToStringConverter}}"
                 IsChecked="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToBooleanConverter}, Mode=TwoWay}" />
    <RadioButton Grid.Row="1"
                 Grid.Column="1"
                 Content="{Binding PreferredUnit, ConverterParameter=Imperial, Converter={StaticResource EnumToStringConverter}}"
                 IsChecked="{Binding PreferredUnit, ConverterParameter=Imperial, Converter={StaticResource EnumToBooleanConverter}, Mode=TwoWay}" />
</Grid>

Does anyone here think I am on the right track with my approach? Any suggestions on improving the code (in terms of logic, clarity, performance)?

Answer 1

This looks perfectly reasonable. In fact, the only thing I would really change is this:

string parameterString = parameter as string;
if ( parameterString == null )
    return DependencyProperty.UnsetValue;

if ( Enum.IsDefined (value.GetType (), value) == false )
    return DependencyProperty.UnsetValue;

You have this in both Convert methods, and it would be just as clear like this:

string parameterString = parameter as string;
if ( parameterString == null || !Enum.IsDefined (value.GetType (), value) )
    return DependencyProperty.UnsetValue;

Additionally, you could concatenate this:

object parameterValue = Enum.Parse (value.GetType (), parameterString);
return parameterValue.Equals (value);

Like this:

return Enum.Parse (value.GetType(), parameterString).Equals (value);

For your XAML, you do not need to put each property on its own individual line. I typically only break them up when they do not fit on my screen all at once, more like this

<TextBlock Text="How do you measure distance?" Margin="6,0,0,0" Grid.ColumnSpan="2" Grid.Row="0" />
<RadioButton Grid.Row="1" Grid.Column="0"
             Content="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToStringConverter}}"
             IsChecked="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToBooleanConverter}, Mode=TwoWay}" />

Finally, for your UI, you most likely want to label your radio buttons as what they are, and perhaps change your UI a little to accommodate the labels and still look good. I would recommend a ComboBox if you have trouble adjusting the UI to look good. You could then bind the SelectedItem or SelectedIndex to a value in your VM and use a converter to change the value as needed.

<ComboBox Text="Label">
    <ComboBoxItem>Metric (meters)</ComboBoxItem>
    <ComboBoxItem>Imperial (feet)</ComboBoxItem>
</ComboBox>

Answer 2

I think it is worth mentioning, that this design decision will bite you back as soon as you start working on localization. There is no easy way to localize attributes, because you can only pass constant string values to attribute constuctor.

If localization is out of the question, then this design is fine, I guess.

Answer 3

While I admit that chances are low that another Unit will pop up, I do feel that your solution is not really following the "spirit" of WPF/Silverlight. Particularly the fact that you code two RadioButtons with virtually the same code doesn't sit well with me. It feels conveluted, especially the enum-to-boolean conversion.

I'd much rather have a List of enums which then binds to a ListBox which exposes radioButtons via a template, as in this SO solution or this blog post.

---

I'm also not a fan of using a Grid like you did; I'd much prefer StackPanels:

<StackPanel Orientation="Vertical">
    <TextBlock Text="How do you measure distance?" Margin="6,0,0,0" />

    <StackPanel Orientation="Horizontal">
        <RadioButton Content="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToStringConverter}}"
                     IsChecked="{Binding PreferredUnit, ConverterParameter=Metric, Converter={StaticResource EnumToBooleanConverter}, Mode=TwoWay}" />
        <RadioButton Content="{Binding PreferredUnit, ConverterParameter=Imperial, Converter={StaticResource EnumToStringConverter}}"
                     IsChecked="{Binding PreferredUnit, ConverterParameter=Imperial, Converter={StaticResource EnumToBooleanConverter}, Mode=TwoWay}" />
    </StackPanel>
</StackPanel>

I find that even simple Grids often introduce complexity where it isn't needed, which is evidenced by Grid.Row and Grid.Column. Because next thing another row needs to be added, and then another, and before you know it you've got ten RowDefinitions; and then one of the rows need to be removed or a new row needs to be inserted in the middle...