Does my code conform to modern conventions?

Question

I've spent a few days making the transition from WinForms to WPF, and do not have much time for tutorials as the work needs to be done quickly. I was wondering if anyone could take a look at a sample of my code and point out any conventions that I'm missing (and that are commonplace in modern WPF usage). I've picked a random sample of some XAML code which I've written:

<Window x:Class=""
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
         Width="300" Height="300">
    <Window.Resources>
        <Style x:Key="buttonsStyle" TargetType="Button">
            <Setter Property="Height" Value="25"/>
            <Setter Property="Width" Value="80"/>
        </Style>
        <Style x:Key="componentsStyle" TargetType="Control">
            <Setter Property="Height" Value="20"/>
            <Setter Property="Width" Value="190"/>
        </Style>
        <Style x:Key="labelStyle" TargetType="Label">
            <Setter Property="VerticalAlignment" Value="Center"/>
        </Style>
    </Window.Resources>
    <StackPanel Orientation="Vertical">
        <Grid>
            <Grid.RowDefinitions>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
                <RowDefinition>
                </RowDefinition>
            </Grid.RowDefinitions>
            <Grid.ColumnDefinitions>
                <ColumnDefinition Width="70">
                </ColumnDefinition>
                <ColumnDefinition Width="200">
                </ColumnDefinition>
            </Grid.ColumnDefinitions>
            <Label Grid.Column="0" Grid.Row="0" Style="{StaticResource labelStyle}">Server:</Label>
            <ComboBox Grid.Column="1" Grid.Row="0" Name="cmbServer" Style="{StaticResource componentsStyle}"/>
            <CheckBox Name="chkWindowsAuthentication" Grid.Column="0" Grid.Row="1" IsChecked="False"/>
            <Label Grid.Column="1" Grid.Row="1" Style="{StaticResource labelStyle}">Use Windows Authentication</Label>
            <Label Grid.Column="0" Grid.Row="2" Style="{StaticResource labelStyle}">Username:</Label>
            <TextBox Grid.Column="1" Grid.Row="2" Name="txtUsername" Style="{StaticResource componentsStyle}"/>
            <Label Grid.Column="0" Grid.Row="3" Style="{StaticResource labelStyle}">Password:</Label>
            <PasswordBox Grid.Column="1" Grid.Row="3" Name="txtPassword" Style="{StaticResource componentsStyle}"/>
            <Label Grid.Column="0" Grid.Row="4" Style="{StaticResource labelStyle}">Database:</Label>
            <ComboBox Grid.Column="1" Grid.Row="4" Name="cmbDatabase" Style="{StaticResource componentsStyle}"/>
        </Grid>
        <StackPanel Orientation="Horizontal" Margin="10,30,10,10">
            <Button Margin="0,0,10,0" Style="{StaticResource buttonsStyle}">Connect</Button>
            <Button Style="{StaticResource buttonsStyle}">Reset</Button>
        </StackPanel>
    </StackPanel>
</Window>

The code is functional and displays the proper result, however I am worried that I am not using the right techniques. Any input at all is greatly appreciated.

Answer 1

All in all, your code is quite well-structured. Here are a couple of suggestions for improvement:

1. Consider moving your styles to a resource dictionary:

   <Window.Resources>
       <ResourceDictionary Source="Resources.xaml" />
   </Window.Resources>
   

   This will de-clutter your file and allow you to re-use the styles in other windows.

2. Collapse empty tags (use the self-closing syntax):

   <Grid.RowDefinitions>
       <RowDefinition />
       <RowDefinition />
       <RowDefinition />
       <RowDefinition />
       <RowDefinition />
   </Grid.RowDefinitions>
   

   ^{(the same goes for the ColumnDefinitions and any other empty tags)}

3. Declare styles that affect all controls of one type without a key:

   <Style TargetType="Button"> ... </Style>
   <Style TargetType="Label"> ... </Style>
   

   That way, you don't seperately have to set all the labels to labelStyle and all the buttons to buttonsStyle, just remove all occurences of the following two attributes:

   Style="{StaticResource labelStyle}"
   Style="{StaticResource buttonsStyle}"
   

4. The default value of Grid.Column and Grid.Row is 0, you can get away without setting it explicitly. And CheckBox.Checked is false by default. So you can remove all occurences of the following:

   Grid.Column="0" 
   Grid.Row="0" 
   IsChecked="False"
   

5. Generally, in WPF, widths and heights are not explicitly stated. You should try to use relative and automatic dimensions where possible (which often requires choosing the correct container). For instance, you could use relative sizing for your grid columns:

   <Grid.ColumnDefinitions>
       <ColumnDefinition Width="0.35*" />
       <ColumnDefinition Width="*" />
   </Grid.ColumnDefinitions>
   

   In other situations, working with Margin can be a replacement for explicit pixel sizes. Remember that hard-coded sizes are an obstacle to localization (where long words in foreign languages are cropped) and less flexible.

These suggestions shrink your Window .xaml file from 54 to 37 lines (Resources.xaml has 14).