views:

160

answers:

1

Hi!

I'm looking for a simplest way to remove duplication in my WPF code.

Code below is a simple traffic light with 3 lights - Red, Amber, Green. It is bound to a ViewModel that has one enum property State taking one of those 3 values.

Code declaring 3 ellipses is very duplicative. Now I want to add animation so that each light fades in and out - styles will become even bigger and duplication will worsen.

Is it possible to parametrize style with State and Color arguments so that I can have a single style in resources describing behavior of a light and then use it 3 times - for 'Red', 'Amber' and 'Green' lights?

<UserControl.Resources>
    <l:TrafficLightViewModel x:Key="ViewModel" />
</UserControl.Resources>

<StackPanel Orientation="Vertical" DataContext="{StaticResource ViewModel}">
    <StackPanel.Resources>
        <Style x:Key="singleLightStyle" TargetType="{x:Type Ellipse}">
            <Setter Property="StrokeThickness" Value="2" />
            <Setter Property="Stroke" Value="Black" />
            <Setter Property="Height" Value="{Binding Width, RelativeSource={RelativeSource Self}}" />
            <Setter Property="Width" Value="60" />
            <Setter Property="Fill" Value="LightGray" />
        </Style>
    </StackPanel.Resources>

    <Ellipse>
        <Ellipse.Style>
            <Style TargetType="{x:Type Ellipse}" BasedOn="{StaticResource singleLightStyle}">
                <Style.Triggers>
                    <DataTrigger Binding="{Binding State}" Value="Red">
                        <Setter Property="Fill" Value="Red" />
                    </DataTrigger>
                </Style.Triggers>
            </Style>
        </Ellipse.Style>
    </Ellipse>
    <Ellipse>
        <Ellipse.Style>
            <Style TargetType="{x:Type Ellipse}" BasedOn="{StaticResource singleLightStyle}">
                <Style.Triggers>
                    <DataTrigger Binding="{Binding State}" Value="Amber">
                        <Setter Property="Fill" Value="Red" />
                    </DataTrigger>
                </Style.Triggers>
            </Style>
        </Ellipse.Style>
    </Ellipse>
    <Ellipse>
        <Ellipse.Style>
            <Style TargetType="{x:Type Ellipse}" BasedOn="{StaticResource singleLightStyle}">
                <Style.Triggers>
                    <DataTrigger Binding="{Binding State}" Value="Green">
                        <Setter Property="Fill" Value="Green" />
                    </DataTrigger>
                </Style.Triggers>
            </Style>
        </Ellipse.Style>
    </Ellipse>
</StackPanel>

A: 

As long as your "Traffic Light" is wrapped up inside a control, which it appears it is, I don't think this is horrible. Each ellipse is well defined and has different triggers, each indicating its own state. You've already factored the common parts out into the base style, which is good.

You could wrap the individual ellipses inside another user control (which wouldn't need a backing ViewModel) that had an ActiveState property and an ActiveFill property. Then your TrafficLight looks something like:

<StackPanel Orientation="Vertical" DataContext="{StaticResource ViewModel}">
    <my:Indicator State="{Binding State}" ActiveState="Red" ActiveFill="Red" />
    <my:Indicator State="{Binding State}" ActiveState="Amber" ActiveFill="Red" />
    <my:Indicator State="{Binding State}" ActiveState="Green" ActiveFill="Green" />
</StackPanel>

This lets you wrap up all your Ellipse styling inside your Indicator control and the only thing that control needs to worry about is comparing the State to the ActiveState to determine if it should fill itself with the ActiveFill brush.

As to if this is worth the effort or not, that depends on how many of these you have floating around and if you use them outside of your Traffic Light user control. Remember: You Ain't Gonna Need It.

Ben Von Handorf
I thought about creating a separate control but hoped there may be some other way of eliminating duplication. Thanks for the answer!
Konstantin Spirin