Thursday, October 23, 2008

How To Write WPF Code That Doesn't Suck

This may turn into a series, we'll see how it goes. This post is inspired by some code I inherited from a well-known design firm.

If you've ever written a WPF application, chances are you've written some XAML as well as some C#. Typically you'd do your UI in XAML and your logic in C#. Event handling, in particular, is usually done in code-behind, especially if you have some complex rules. Consider the following (pardon the formatting):

private static void SomeUserControl_DragLeaveOrDrop(object sender, DragEventArgs e)
{
if (VisualTreeHelper.GetChildrenCount((DependencyObject)sender) > 0)
{
Grid grid = (Grid)VisualTreeHelper.GetChild((DependencyObject)sender, 0);
Storyboard sb = grid.TryFindResource(SomeAnimationResourceName) as Storyboard;

if (sb != null)
sb.Begin(grid);

}
}

What's wrong with this? The answer, like most bad things, is a four-letter word. Everything will compile just fine, regardless of whether Child 0 of the sender is a Grid or not. But when this method gets invoked, you'll get an InvalidCastException.

Worse yet, the way this particular code was wired up, you wouldn't see this method in the call stack. Depending on how your IDE is set up, you might see [External Code] in the call stack. This can be very tricky to track down for a developer such as myself who thinks he's doing a simple UI update.

How to improve this? I have several suggestions:

* The animation is defined in Xaml, why not invoke it from Xaml? Triggers are wonderful things. This would be easy to implement for the method I've provided, but it looks like this code was just copied and pasted from another spot, where there was more complex logic validating whether or not the animation should fire. A custom converter could handle that, but at that point you're writing C# anyway, so...
* If you're going to make a blatant assumption about types in code, the least you could do is throw a more meaningful exception. The problem here is, as I mentioned, the source of the exception wasn't readily apparent, and this would presumably apply to a more meaningful exception as well.
* My preferred solution in this case: BE LESS SPECIFIC. What benefit are we getting from the Grid? TryFindResource() is defined on FrameworkElement, which Grid inherits from. Storyboard.Begin() accepts a FrameworkElement. Changing the highlighted code above to "FrameworkElement" gets us the same result, plus the code won't break when a naive developer comes along and updates the Xaml to use something other than a Grid.

0 comments: