Building What's Next

Static analysis is a technique that analyzes the source code (or compiled object code) of your software. Static analysis tools can help you improve your code quality by alerting you to design issues, performance problems, and maintainability constraints.

These tools are designed to be integrated into the development process, and we have baked it into ours.  Team Foundation Server even lets you enforce check-in policy so that code that fails FxCop cannot be checked in. We don’t go that far, but every developer is asked to run code analysis before checking in. It’s a key part of our code review process.

My goal here is not to get developers to blindly change code to make the warnings go away. Rather, each warning that comes up is a change to learn something.

Here is some C# code that will trigger a common warning: Do not cast unnecessarily.

void start_Click(object sender, EventArgs e)
{
   Size controlSize = ((Control)sender).Size;
   RightToLeft rightToLeftValue = ((Control)sender).RightToLeft;
   Control parent = (Control)sender;
}

This code casts the same object variable to Control three times. Casting is slow, so this will have a small negative impact on performance. This can easily be fixed by casting the parameter once and storing it in a local variable.

You may be thinking that this is going to be a negligible speed increase – and you would probably be right in most cases. However, it’s not just about saving those two unnecessary casts. By seeing this warning and learning how to fix it, the developer will think about this in the future as well. The performance boost lives forever!

Here’s another one of my favorites: Do not catch general exception types.

try
{
   outStream = File.Open(outFile, FileMode.Open);
}
catch (Exception e)
{
   Console.WriteLine("Unable to open {0}.", outFile);
}

This is in big bold letters in our coding guidelines document, but it still comes up frequently. The code as written here would catch any exception – even exceptions that have nothing to do with the file operation. It’s important to catch a more specialized exception class – in this case IOException would be a good choice. Wherever this code is in your application, it should only be handling IO-related exceptions.

There is certainly a time and place to catch Exception and other general types of exceptions – and that’s when it’s appropriate to ignore these messages. The important part is that you take the time to decide what is the right time, and discuss it with others on the team.

The most frequent objection to using these tools is the noise level. Admittedly, these tools can produce a lot of warnings, especially when running them on pre-existing code. Make the investment! As you bake it into your process, you’ll build up a list of messages to mark as ignorable, and the signal-to-noise ratio will go up day by day. The payback is two-fold; not only do you get more robust code in the project you’re working on, but it’s another way for your team to learn good development practices.

There are great static analysis tools out there for every platform.   For .Net programmers, it’s built into Visual Studio Team System, or you can download FxCop for free. If you’re on a Java project, Checkstyle is a good place to start. Whichever platform you’re working on, at least give these tools a shot.