Coding and Development Standards: 15 Do’s and Don’ts for My Subcontractors

I am an independent software development consultant. By independent, I mean I generally work directly with my clients, rather than going through a contract agency or other firm. When I have more work than I can take on myself, I sometimes hire subcontractors to help out.

To help get new subs up to speed on projects and my development environment, I created documents addressing various things they need to know. For instance, one document explains how to connect to my Subversion server, while another one lists various required or otherwise useful tools. There is, however, another less obvious problem – actually a myriad of small problems – with taking on a new developer.

When I review a subcontractor’s check-in or observe some aspect of their work, I frequently encounter things that are not done “right.” I put “right” in quotes because I fully recognize that some, perhaps most, of these issues are just preferences. Some are actually best practices, but  many simply reflect the way I like things done. So I finally put together a set of guidelines to inform subcontractors of how to do things “my way.”

Do’s and Don’ts

As I started to write my guidelines, I had to figure out how to present them in an easy-to-read list, using consistent language. I settled upon making each item a Do or Don’t. This format works because some things are behaviors I want the subcontractors to adopt and others are practices that I want them to stop.

Before anyone comments telling me why my way is wrong, let me re-iterate that these are my preferences, and not a list of absolutes I expect everyone to follow. The only reason I post this in the first place is in to offer some advice for anyone wanting to get another perspective on these areas. Also if you find my preferences fairly similar to your own ideas, you can use this as a starting point for writing a document for your own employees or subcontractors. Lastly, this is as good a place as any to keep this list and I can just refer my subs to it.

The list has two main sections (at least for now).  These are Source Code Control and Coding Standards.  I may add more in the future but all of my issues so far were able fit in to these two categories. I should also note that I predominantly work in .NET, so some of the coding standards are specific to that environment.

Source Code Control

• DO make sure the checked-in source will always build.  Before checking in you should always synch to the latest source and perform a clean build to make sure nothing is broken.[Note: There are some rare exceptions to this rule. For example, certain kinds of changes, like file moves/renames and other restructuring tasks, require a series of check-ins to complete.  In these cases you inform everyone else beforehand what change is taking place and when.  Then, when you make the changes, include clear comments indicating that a given check-in is part of a multi-phase check-in.  Also, if for this or any other reason, you are knowingly checking in a set of changes which will break the build, add something like "!!! Broken Build !!!" to the first line of your check-in comment so it is clear to any reviewing the change logs not to sync with this revision.]
• DO add comments to every single check-in. Try to make the first line summarize the check-in as that is the only line which shows in the Show log dialog, but add more details as well.
• DO review all changes before check-in.  Every time I check-in changes, I bring up the Commit… dialog, review the list of files to make sure I am not checking in anything I don’t want to, nor missing anything I need to add.  Next, I systematically double-click on each and every file that I touched to make sure the changes include only the changes that should be checked in.  Using this process I frequently encounter small sections of code that I added for testing or debug purposes that should not be checked in.  I occasionally see outright problems that would break the build if did check it in.
• DO check for new files to add to the repository.  This is one of the most common mistakes made when checking in changes, forgetting to add important new files that you added to the project.  Everything will still build fine on your machine but you will break everyone else that syncs to your changes.
• DON’T check-in built binary files.  I have seen way too many projects that have all of the .dll, .exe, and .pdb files for a project checked in to source code control.  I have even seen projects with all of the files in the obj folders checked-in.  These files are all built every single time you build the solution on your machine and should not be stored in source code control.  Period.
• DON’T check-in machine/user-local configuration files.  In particular, don’t ever check-in any .suo or .user files.  Similarly, don’t check in *.Publish.xml, *.FileList.txt or *.FileListAbsolute.txt files.
• DON’T check anything in to the project or solution folder(s) not required to build and run the solution.  Don’t check-in any files used to create graphics (.psd, .pdn, .fla, etc.), any log files, temp files, or anything else that is not required to build the solution.  There should be a separate folder outside of the solution’s folder tree for checking in graphics sources, design documents and other project collateral, but these should not be in the actual solution folder hierarchy.

Coding Standards

• DON’T check-in code with build warnings.  Don’t ever check-in build warnings you have created.  The problem with build warnings is they sometimes are actually problems you need to deal with, and if you have a bunch of build warnings already, then you don’t notice when new ones are introduced.  If you are working on a project with legacy code that produces many build warnings, then know how many are reported when you check-out/update, and keep track of that number so you can prevent adding more.  If you can, clean some up some legacy warnings with each check-in so over time the number is reduced to zero.
• DON’T use exceptions for normal code flow.  Having exceptions thrown during normal operation of the code creates “false positives” when you are trying to debug real exceptions.  Very annoying.  Exceptions are also expensive, so it is generally a bad practice to rely on them.  It is important to catch exceptions in many cases to handle possible failure conditions (like making web service calls, doing file i/o, and such), but for things like converting strings to numeric or other values, there is always a non-exception producing way. 
• DO use String.IsNullOrEmpty() rather than stringVal == “”.  There are some very rare cases where you need to check if a string is null separate from if it has any data in it, but in the vast majority of cases you can use String.IsNullOrEmpty(), even if you already know it is not null. 
• DO use String.Empty instead of “”.  My code pretty much does not use “” anywhere.  It is always String.Empty.  If the value of String.Empty ever changes (unlikely, but possible), then all of my code will consistently change with it.
• DON’T use string concatenation, instead always use String.Format().  It pays to be consistent, String.Format() supports fairly rich formatting capabilities. It is way easier to re-arrange the contents of text using String.Format() than mucking with concatenated strings, and if there is ever any chance of code being globalized/localized, you will need to switch all string concatenation to String.Format() anyway.
• IDisposable (despite being last on the list, this is very important!)
  • DO understand IDisposable and be aware of when it is used.  There are many classes in the .NET framework which implement IDisposable and therefore must be disposed.  The #1 best way for doing this is with the “using” (in C#) or “Using” (in VB.Net) statement.  If you can at all avoid it, don’t hang on to disposable objects, instantiate them with “using”, make use of them, and then let them be disposed when the “using” statement falls out of scope.  The next best way to ensure disposal is via try/finally.  If you have to hold on to a disposable object as a member variable, the next bullet point applies.The most common place that IDisposable requirements are abused is in ADO.NET objects.  Many of the data access objects implement IDisposable and therefore invoke the two rules above.   Read an MSDN article titled Implementing a Dispose Method for some explanation of IDisposable.  There is also an MSDN Magazine article, Digging into IDisposable.
  • DO know the IDisposable interface implementation pattern and requirements.  If you encapsulate (have as a member variable) any object which implements IDisposable, then your class needs to also implement IDisposable, and any code calling your class must dispose your object which will, in turn, dispose the object it is holding on to.
  • DO use the using statement to ensure an object is disposed.  Both C# and VB.NET support the using statement for ensuring IDisposable.Dispose is called.  Always prefer using to try/catch if at all possible.