Monthly Archives: September 2009

I Joined a Project for Fun

Posted on by

User ident on the XVBT forums asked for some comments and suggestions about his project.  I took a look at it and decided the number of comments sans explanation of fixes would exceed the size of a post, so I asked if I could join the project and make some lessons out of refactoring it.

The project itself is a neat little tool for automating ImageShack uploads.  On the outside, it looks pretty slick.  After inspecting the code, I have a bad feeling that it doesn’t work anywhere near as slick as it looks, but that’s why I’m here.  The application is an almost perfect example of what WinForms development does to suppress good software design practices.  In this ~3,000 line application, 2,600 of the lines are in ClientForm, the main form of the application.  This form is responsible not only for UI interaction, but all of the application logic.  There’s many other errors that are more related to what happens when a novice writes a large application than what WinForms inflicts upon you: dozens of unused methods, huge areas of commented-out code, no consistent naming conventions, etc. There’s at least a wrapper for the ImageShack API, but I haven’t peeked inside of that yet to see if it needs some work as well.  It looks like it was lifted from a blog by a man named Bryce Thomas, then converted from C# to VB.  I’m not sure if any attribution is currently given; if not I’m going to fix that.

The code for the client itself is not under any particular license, but does happen to have this notice at the top of each file:

'*************************************************************
' 
' DX Coders
' Copyright 2001-2009 Simple Coders
' All Rights Reserved 
' Created: 09/06/09                                   *
' Author: Johnson

' NOTICE:  DX permits you to use, modify, and 
' distribute this file in any way.   
' DX takes no responsibility on how you use 
' this code. It is strictly for learning purposes
' and is not designed to cause harm. If you decide
' to reuse this code please give credit where credit is due.
'*************************************************************

I have entertained the notion that the developer(s) (there is a mysterious second developer involved that I’ve never met) are planning to have me clean up their application for free, then cut off my access to the project and sell it.  I would be disappointed if this happens, but I decided that the experience of reworking an application from a mess into something polished is something I need more than defense against my hobby work benefiting someone else.  The comments above mean that, should they excommunicate me from the project I have no qualms about releasing the source code and promoting the project as a fork under a very permissive copyleft license.

I have one concern at this point.  The code looks like it’s either a mishmash of examples gathered from various forums or the result of running a C# to VB .NET converter on a pre-existing project.  Indeed, one of the more questionable bits of code is admitted to be the result of running a converter on some C# example code.  If you have written an ImageShack upload client and this one looks familiar, please contact me with proof.  I will not work on a project that was ripped off.

As I work on refactoring, I’m going to make posts that detail why I did what I did.  I’m going to cross-post these onto the blog.  I’d normally do them *only* on the blog, but I don’t want to undermine XVBT’s policies by taking discussion of a user problem offsite.  I’m already skirting the edge by taking on a full project.

UI Control Naming Conventions

Posted on by

Microsoft was kind enough to publish a set of design guidelines for developing class libraries, and I use the book Framework Design Guidelines that is based on them daily. This has settled many disputes over the conventions our team should follow, and it’s the basis of many suggestions I make on the XVBT forums. The guidelines help lay out the practices that the .NET Framework team tries to adhere to when designing types. If everyone adheres to the guidelines, then the entirety of the .NET ecosystem will be consistent and usability will increase.

The guidelines have an odd position in the documentation. I find that most of MSDN and Microsoft’s other efforts are focused on helping the application developer. Contrary to this position, the design guidelines are focused on developers of class libraries that will be used by other developers. This is what I do, so it’s great for my demographic to get some love. However, the lack of focus on application development means there’s some important guidelines that aren’t there.

Chapter 3 of the books involves naming guidelines; you can follow along sans commentary online. The chapter discusses capitalization conventions and how to name every aspect of your code from variables to interfaces to DLLs. One omission that would be useful for application developers is guidelines for naming controls in applications.  I’ve experimented with 3 approaches, and each has its disadvantages.

1. Hungarian Notation

If this is established as the guideline, Hungarian notation is used for each control. That is, a text box for name input becomes txtName and a list view for displaying songs becomes lvSongs. Optionally, a leading underscore might be used, but I tend away from it when using this convention.

One benefit of this notation is familiarity. VB6 developers in particular favor this approach because it follows the generally agreed-upon conventions used in that language.  Another benefit is an increase in understandability.  When you see txtName, it is clear that this is a text box that represents a name.

One drawback is the naming guidelines use a strong DO NOT guideline to condemn Hungarian notation.  Nothing else in .NET uses Hungarian notation. This makes applications that follow the notation seem like they stick out a little. Another drawback is the notations aren’t really standardized. Most people agree on txt for TextBox and lv for ListView because these controls have been around forever, but newer controls like TableLayoutPanel and Grid don’t have agreed-upon prefixes. Also, how do you decide what to use for custom controls, UserControls, and third-party controls? One more drawback is this notation makes code brittle to change. Maybe your prototype starts with a TextBox but later you switch it to a NumericUpDown. This means you not only have to change the control and any logic that worked with it, but you also have to hunt down all references to the variable and rename it. (I admit I’m a relative newbie to Presentation Model patterns and it could be that this is less of a concern in those since the form would likely expose the control as a typed property, but I think it’s still applicable.)

To me, the drawbacks seem to outweigh the benefits. However, this is the convention I learned when I started in VB .NET many years ago, so I find it most comfortable and no matter how many arguments against it I hear, it’s the one I use.

2. Suffixes

When following this guideline, you pick a descriptive name and suffix it with the type of control. Use of camelCase or PascalCase is disputed, but generally I see PascalCase used more prominently with this technique. Using the examples from above, we’d use NameTextBox and SongsListView.

This has practically the same benefits and drawbacks as Hungarian Notation. You’re still identifying the type of control alongside the purpose, and it’s still clear from the variable name what kind of control is at work. This is technically a form of Hungarian notation, so still falls afoul of the DO NOT guideline. The problem with standardized prefixes is eliminated: since you use the full type name there’s no need for agreement. This technique is still brittle if you change controls.

Personally, I find that this convention clashes with how I think about the UI and makes it harder for me to find the variable, but this is probably because I’ve used Hungarian notation for years. I’ve tried following this guideline, but I find that more often than not I forget if I chose ItemCount or NumberOfItems as the prefix, and knowing the suffix doesn’t help me find it. Perhaps my naming discipline is poor, but it just doesn’t work for me.

3. Plain Old Variable Names

When following this guideline, you pick a descriptive name for the control and use that. There’s dispute over casing and usage of a leading underscore, but I find that two camps are the most prominent: PascalCase and _camelCase. Using our example controls, we would use Name and Songs or _name and _songs.

There are a few benefits to this approach.  One benefit is you aren’t using Hungarian notation so you aren’t falling afoul of any DO NOT guidelines. Another benefit is the code is no longer brittle if you change the control type.

Despite the benefits, there are many drawbacks to this approach no matter what casing convention is followed. If we’re writing a custom control or a custom UserControl that needs a text box for a name, then Name is already taken by Control.Name and we’ll have to pick a different name like PersonName. This can lead to accidental use of Name when you meant PersonName, and since you aren’t using the first name that came to mind you’re going to spend a lot of time trying to remember what your 2nd choice was. In addition, this casing convention is used for properties and constants, so if you have many of those your controls will be buried among them in Intellisense. This is why many people use the _camelCase convention, but since this is a very common convention for private fields it’s not much better.  Is _songs the control that represents the songs or a collection of songs? If you’ve got 20 properties with backing fields and 5 controls on the form, you have to wade through 25 entries if you can’t quite remember the correct name. _camelCase also looks particularly heinous when used as the x:Name attribute in XAML.

I find I am always dissatisfied when I use this convention.  If I use PascalCase then I almost always get the controls mixed up with constants. Even more often, the control name clashes with a property that I want to have and I end up having to pick a worse name for the control.  Then, I can’t remember the name I picked.  If I use _camelCase then the controls are buried among property backing fields, and name clashes still happen.  If _songs is the control that represents a list of songs and I want a Songs property, what do I do for the backing field? (There’s a few answers that involve generating the property value in different places, but most of them wouldn’t occur to a novice.) No matter how I try to follow this convention, I don’t like it.

I’m sure there’s other approaches I haven’t thought of, but these are the three I’ve tried. All of them have drawbacks that seem to outweigh the benefits, but I side with Hungarian notation out of familiarity. If there were guidelines to follow, I’d settle on the method they suggest, but there are none so I have a hard time deciding if I’m right.  What do *you* do?