8
\$\begingroup\$

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)?

\$\endgroup\$
8
\$\begingroup\$

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>
\$\endgroup\$
  • \$\begingroup\$ Thank you for the answer. Regarding the line breaks, I think its more of personal choice. This way its easier for me to read as each property will be on its own line. \$\endgroup\$ – Adarsha Jan 14 '15 at 1:14
  • \$\begingroup\$ Could you please explain more on the "you most likely want to label your radio buttons as what they are" part. For the Radio Buttons, I am already getting the labels from model from its description. Do you mean I need to change the description or just hardcode it in the XAML? I don't want to go for comboboxes as the are not easy to use on a Phone. I might actually just use a toggle button, and have two labels (bound to descriptions) at either end. \$\endgroup\$ – Adarsha Jan 14 '15 at 1:40
  • \$\begingroup\$ True, I am experimenting with ToggleSwitchButton(from the toolkit) and ComboBoxes, need to finalize. \$\endgroup\$ – Adarsha Jan 14 '15 at 2:17
  • \$\begingroup\$ I finally ended up using ToggleButton (BaseClass for RadioButton) which plays very nicely for this kind of needs. It gives lot of "clickable" real estate for finger friendly toggles. \$\endgroup\$ – Adarsha Jan 16 '15 at 2:44
3
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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...

\$\endgroup\$
  • \$\begingroup\$ I don't agree that ListBox is the way to go. ListBox was suggested as rather a hack before .Net 4.0 when IsChecked binding was broken. In fact WPF team internally uses[similar converter][1] (Search for enum). Only change in my way is missing x:static as its not supported in WP8. I considered using Grid as I wanted both horizontal and vertical layout control. Nested Stackpanel vs one Grid should give similar performance, right? [1]blogs.msdn.com/b/jaimer/archive/2009/09/22/… \$\endgroup\$ – Adarsha Jan 13 '15 at 17:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.