Custom Control Development: Simple code guidelines
Great custom control development is a skill that takes more than time and experience to perfect. It’s also not a widely documented practice. When it comes to Silverlight controls, there are similarities and differences from WPF custom control development, too – so that chapter on controls in your favorite WPF book often is not directly applicable.
While working with teams throughout Microsoft, and trolling Silverlight forums on Silverlight.net and StackOverflow, I’ve come across a few situations where I’ve wanted to share some guidelines for custom control development. Over the next few posts, I’d like to occasionally share my thoughts on this topic, and whatever tips seem pertinent at the time of posting.
I do admit that there’s a lot of flexibility in control development, so I’ll be basing a lot of my tips on both official and unofficial practices on the Silverlight Toolkit team. And I understand if you don’t agree with everything I have to say. But I do hope this information will be useful!
Contain your classes
Controls and classes should always be in their own unique file, instead of having multiple controls or classes in a single file. This includes enums and structs, etc.
So the Dock enum belongs in a file called Dock.cs.
Typically classes are broken up into folders of shared features or sub-namespaces.
Keep your regions under control
Regions that serve no real purpose have no need in your code. Grouping all “Public Methods" into a region and having another region with “Private Methods” isn’t helpful to most people.
However, grouping related concepts, interface implementations, and complex algorithms into regions can be a great way to make that 2,000 line file feel a little more navigable.
We always place DependencyProperty declarations and change handlers inside clear regions.
Be consistent about where fields belong
Fields should always be private, only accessible to the control implementation. Whether you place them at the top of the file, the absolute bottom, or alongside related methods and properties (such as in a #region for a DependencyProperty), is a choice that should be consistent within a codebase.
We often try and place backing fields near their respective properties.
Break comments at 80 characters
By maintaining a visual break for comments, they become easier to digest while looking through code, regardless of monitor resolution.
To help with this, you can use a nice registry hack to place a visual guide in the Visual Studio editor (pictured in the above screen capture). Here’s the registry key to set (you can change the color and/or character placement as well):
[HKEY_CURRENT_USER\Software\Microsoft\VisualStudio\9.0\Text Editor]
"Guides"="RGB(220,220,220) 80"
Consider writing great XML comments
By writing “great” XML comments, as they would be written in programmer documentation by a writer, you’ll save the time for anyone documenting the controls, improve the IntelliSense experience, and over time find that your code is very clear and crisp.
You can even consider using SandCastle for generating MSDN-style documentation from your code in this case.
If you find yourself writing too much in the comments, your method or feature may be too complex – consider simplifying.
The style we use on the Silverlight Toolkit consists of this:
- Comments end with a period
- Properties start with “Gets or sets”, or “Gets or sets a value indicating whether”
- Properties that are only a getter only have the “Gets” portion of “Gets or sets”, etc.
- Constructors use wording such as “Initializes a new instance of Type.”
Here’s a set of example comments borrowed from the Silverlight team…
Class comments:
/// <summary>
/// Represents a control that displays hierarchical data in a tree structure
/// that has items that can expand and collapse.
/// </summary>
public partial class TreeView : ItemsControl, IUpdateVisualState
{
}
A private field:
/// <summary>
/// A value indicating whether a read-only dependency property change
/// handler should allow the value to be set. This is used to ensure
/// that read-only properties cannot be changed via SetValue, etc.
/// </summary>
private bool _allowWrite;
A public property:
/// <summary>
/// Gets the selected item in a
/// <see cref="T:System.Windows.Controls.TreeView" />.
/// </summary>
/// <value>
/// The currently selected item or null if no item is selected. The
/// default value is null.
/// </value>
public object SelectedItem
{ get; set; }
An event handler method:
/// <summary>
/// SelectedItemProperty property changed handler.
/// </summary>
/// <param name="d">TreeView that changed its SelectedItem.</param>
/// <param name="e">Event arguments.</param>
private static void OnSelectedItemPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{ /* ... */ }
Public event comments:
/// <summary>
/// Occurs when the value of the
/// <see cref="P:System.Windows.Controls.TreeView.SelectedItem" />
/// property changes.
/// </summary>
public event RoutedPropertyChangedEventHandler<object> SelectedItemChanged;
Constructor:
/// <summary>
/// Initializes a new instance of the
/// <see cref="T:System.Windows.Controls.TreeView" /> class.
/// </summary>
public TreeView()
{
DefaultStyleKey = typeof(TreeView);
ItemsControlHelper = new ItemsControlHelper(this);
Interaction = new InteractionHelper(this);
}
On template application:
/// <summary>
/// Builds the visual tree for the
/// <see cref="T:System.Windows.Controls.TreeView" /> control when a new
/// control template is applied.
/// </summary>
public override void OnApplyTemplate()
{
base.OnApplyTemplate();
}
Bool property with getter and setters:
/// <summary>
/// Gets or sets a value indicating whether the drop-down portion of
/// the control is open.
/// </summary>
/// <value>
/// True if the drop-down is open; otherwise, false. The default is
/// false.
/// </value>
public bool IsDropDownOpen
{
get { return (bool)GetValue(IsDropDownOpenProperty); }
set { SetValue(IsDropDownOpenProperty, value); }
}
Your event handlers should be thread-safe
Firing events should be done by first storing the handler in a local variable, similar to this:
public event PropertyChangedEventHandler PropertyChanged;
private void OnPropertyChanged(string propertyName)
{
PropertyChangedEventHandler handler = PropertyChanged;
if (handler != null)
{
handler(this, new PropertyChangedEventArgs(propertyName));
}
}
Fire control events on the user interface thread
Silverlight only have a single UI thread. However, if you’re interacting with the network or worker threads, the users of your control should not have to worry about being on the UI thread.
Expect that all your events will likely end up changing UIElements that will be checking the thread.
Here’s a simple, efficient way to make sure that your events fire on the UI thread:
/// <summary>
/// Occurs when the DownloadProgress property has changed.
/// </summary>
public event RoutedEventHandler DownloadProgressChanged;
private void OnDownloadProgressChanged(object sender, RoutedEventArgs args)
{
if (!Dispatcher.CheckAccess())
{
Dispatcher.BeginInvoke(() => OnDownloadProgressChanged(sender, args));
return;
}
var handler = DownloadProgressChanged;
if (handler != null)
{
handler(this, args);
}
}
Miscellaneous C# things
- Don’t qualify members using “this” or “base” unless required to disambiguate
- Prefix private fields with an underscore
- Feel free to use automatic properties, but do not use private automatic properties.
- Use lowercase ‘string’ when calling methods on it such as ‘string.IsNullOrEmpty’, instead of ‘String.IsNullOrEmpty’.
- Similar, refer to and use “object” instead of “Object” for locks and other references to generic objects
Organize your Using statements
Instead of placing fully qualified type names with namespaces (System.Windows.Controls.Button is not great – just add a using statement for System.Windows.Controls and refer to this as Button), use the built-in Using statement refactoring support in Visual Studio:
You want your using statements to be the proper subset of namespaces, and in alphabetical order.
Mind the .NET Framework Design Guidelines
When you’re building custom controls, you’re designing a rich API that will enable interesting scenarios for your target developers.
The .NET Framework Design Guidelines book by Krzysztof Cwalina and Brad Abrams is a really important reference that should be regarded to help assist in the design of your framework.
Depending on the target audience of your work, this may be more or less important: an internal development project may not have the same goals as a custom control that you’d like to sell to other developers.
If you see a specific pattern in existing Silverlight or WPF controls, there’s precedent to use a similar implementation in your own controls for the convenience of developers used to the platform.
Off hand, you should consider some of these tips from the book:
Remain CLS Compliant
So that you don’t shut out VB.NET and other language users from the control API, make sure to remain CLS compliant.
This typically translates into situations where you want to expose a generic collection interface instead of an array, for instance. Don’t use the same name, differing only in case.
Good, compliant code:
public IList<interestingitem> InterestingItems { get; }
Code that shouldn’t be exposed in a custom control with public visibility:
public InterestingItem[] InterestingItems { get; }
Naming guidelines
All .NET components typically use PascalCasing for properties, members, and events (PMEs). You’ll find camelCasing in use for parameter names only.
You’ll want to be careful about the use of abbreviations and acronyms, and how compound words should appear in your programming interfaces.
What’s next?
Let me know in the comments what you think, and especially if you have any particular control development topic I should cover in a future post. Hope you like this!
Great post, but I am confused by your event firing. Here’s your code:
public event RoutedEventHandler DownloadProgressChanged;
private void OnDownloadProgressChanged(object sender, RoutedEventArgs args)
{
if (!Dispatcher.CheckAccess())
{
Dispatcher.BeginInvoke(() => OnDownloadProgressChanged(sender, args));
return;
}
var handler = DownloadProgressChanged;
if (handler != null)
{
handler(this, args);
}
}
Two questions:
- Why not just fire the event in Dispatcher instead of calling yourself?
- Why do you get a local copy of the handler to check for null? Why not just check the handler directly?
Nice post Jeff. I agree with most of your points. On the topic of regions, I tend to flip flop on that issue. I think what you say is absolutely correct in that regions should be used to group related concepts, though in much of my code I tend to group public methods / private methods / etc as I find it more useful that way. When classes get too big and contain different concepts to me that’s a sign that refactoring is in order (though that’s not always a possibility I know). With prefixing private variables with an underscore there seems to be varying opinions on that. While I had left hungarian notation back in VB 6 years ago, I did prefix my private fields with m_ until pressured otherwise. So I simply use camel case names for private variables with no prefix, but I miss the prefix (which often means when there is a local variable of the same name I have to use this. to access it – yuk). I see little difference between m_ and simply _ as the prefix, but anyway. Such a small thing such as a prefix can create a big debate. I’m a little confused though with your examples for Remain CLS Compliant. I’m pretty sure I know what you mean from your description – but I don’t get the examples you’ve provided, especially the IList part being ‘good’. Am I missing something?
Chris Anderson
@Shawn,
1: To minimize pressure on the dispatcher, only using it to invoke when necessary.
2: I was always taught the importance of this to avoid race conditions/another thread from nulling out the event handler, though most folks don’t seem to worry about it much. Covered on StackOverflow here more or less: http://stackoverflow.com/questions/786383/c-events-and-thread-safety
@Chris,
On regions and grouping: specifically as it relates to control design, there’s often a few thousand lines of misc. control “goo”: dependency properties and their change handlers, events, and general template code that is going to be difficult to refactor as you describe. As a result I’d rather have control split into logical groupings – give me my dependency property declaration plus change handler, and even backing field, in a region, instead of splitting them into “Fields”, “Dependency properties”, and “Private static method” regions, for instance.
Good advice on the rest though – esp. naming. I guess I’m just sharing my perspective here, coding style and guidelines are a holy war for some, so “When in Rome” is probably the best advice – follow peers and the team’s practices and not some dude’s blog
For the IList vs array: Perhaps this isn’t the best example, since an IList indicates need for dynamic Add/Remove capabilities, something an array won’t have. I suppose the better advice is to return an IEnumerable before an array, if possible.
Nice list. There are however at least two points that will cause StyleCop to choke:
- Grouping DPs in regions will cause it to protest because you don’t follow the rule of ordering methods and properties.
- Stylecop requires the use of “this”. I am not a big fan of using this in front of local params/methods/properties, but started doing it because I am tired of the Stylecop warning. I use Resharper to format my code, so it’s not a big deal.
I went back and forth on the use of Stylecop. The best solution would be to be able to ceate our own rules (like FxCop allows) or at least to switch some rules off. Unfortunately it is not possible.
Cheers,
Laurent
@Laurent,
StyleCop isn’t as customizable as FxCop is (no custom rules), but you can turn off some StyleCop rules as you find necessary. Our settings file disables the ordering and “this” requirement for instance.
More details here: http://blogs.msdn.com/sourceanalysis/pages/sharing-source-analysis-settings-across-projects.aspx
I’ll send or publish the settings file we use on the Silverlight Toolkit if you like, btw.
Hey Jeff,
Like I told you on Twitter, I cannot remember why I deactivated some of the rules, but left the “this.” rule. I am reconsidering this.
For the ordering, I do like to order my methods (I use Resharper’s functionality to do this) however I wish I could enforce ordering for all members except those belonging to a DP (accessors, private attribute, etc…). Failing to do so, I decided to just avoid using regions altogether.
Cheers,
Laurent
Thanks for the guidelines Jeff – I will certainly start using some of the ones that I have not been using.
I am all for any guideline that helps me to write more reliable and easier to maintain code.
One of the things that I do which I don’t see many other developers doing is using prefixes to help identify a variables scope. I love Intellisense; however, one simply does not mouse over ever variable while developing – it can be very easy to assume a variable has one scope when it actually has another which can be disastrous.
The specific method for indicating scope is a question of taste but I do think it is important to do – for example, some of the scope prefixes that I use are: m_ – fields, p_ – passed in parameters, s_ – statics, etc.
To me when I look at a variable I want to instantly be able to tell it’s scope, it’s type, and a clear description of it’s purpose – yea, I know Intellisense will tell me a lot but like I said, we simply do not mouse over every variable and it is very easy to assume the wrong thing.
Anyway, it’s just my style.
Thanks for the guidelines,
David
Hi,
Nice post. But I think that the most critical is control flexibility, customization and easy to use. So there should be some sort of guidelines declare rules to make control flexible, customizable and easy to use.
Let’s take a look at DataForm control as an example:
1. Why dataform holds buttons (add,delete,next,prev) inside it? It is much more simpler to declare that buttons in view and have full control under them. Dataform take collection of items instead of single item. It is very easy to implement next and prev feature without build in support. All this make the control very complex. It has thousands of codelines. Is it easy to use? NOT!
2. Dataform hardcodes generated controls for displaying fields by their data type. What if I want to display datetime with TelerikCalendar? What if want to display int with Slider for propertyX and with textBox for propertyY? As a result form autogeneration is useless in most of cases.
Thanks,
Alexey Zakharov.
Maybe it’s just me, but I don’t see a CheckAccess method in Silverlight. As far as I can tell, this is a WPF thing:
See:
http://msdn.microsoft.com/en-us/library/system.windows.threading.dispatcher_members(VS.95).aspx
vs:
http://msdn.microsoft.com/en-us/library/system.windows.threading.dispatcher_members.aspx
@Morten,
It’s there – it just isn’t visible in IntelliSense, thanks to the EditorBrowsable attribute with the Never state applied.