Building maintainable code with Visual Studio 2008 Code Metrics

Today I decided to do some refactoring on my code, with the main purpose of improving maintainability. As most of you probably know Visual Studio provides some nice tools to indicate which code could use the most improvement. Since Visual Studio 2005, several editions contain Code Metrics. As you run code metrics however, it can be hard to determine how to improve the indicated parts of the code, so that it's Maintainability Index (MI) goes up. In this article I'll try to give some insight into what makes up the MI and how this effects your code.

Let me start by saying that this is in no means a complete and/or exact explanation. The main purpose of this article is to give the insights needed to improve MI numbers on your code.
Second, Microsoft seems to think that an MI of 20 or more is enough. I don't agree and advice any professional development team to set a value that is way higher than this. I've never even written a method that scored lower then 50 and these methods are still not the most clear methods (using recursion, not placing any comments and involving LINQ in a 50 line method).

Also, don't get caught up on this one method in a hundred that you can't seem to improve. If you have a good reason to write a method that still doesn't meet your minimal MI requirement, place some comment stating the reason and leave it alone.

The main components
The MI is made op of several other metrics:
- The Halstead Volumne (HV)
- The Cyclomatic Complexity (CC)
- The number of lines of code in the module (LOC)

Some universities (the Vrije Universiteit in Amsterdam and the Carnagie Mellon University, at least) argue that the percentage of comments in the module should also be included in the calculation of the MI. Unfortunately FxCop calculates the MI based on the IL code, so comments are not taken into account.

So what counts the most in calculating the MI? Well in FxCop, the LOC is actually the most important, reasoning that the less lines of code you have, the easier it is to maintain that code. I would agree to this. The second most important value is the HV, which I will explain in some more detail later.

And to what rate are the LOC and HV more important to the CC in FxCop?
The LOC actually makes up for about 75% of the MI, the HV accounts for about 24% and the CC accounts for the final 1%.
(You can find the actual calculation here)

A close look at the Halstead Volume
As this makes up about 24% of the MI metric, let's have a closer look. I won't bother you with the complete details, as this article is already long enough.

Basicly the HV is based on the assumption that all code is made up of operands and operators.
The calculation involves both the total number of operands and operators and also the distinct number of operands and operators.

Operands are identifiers, types and constants and operators are basicly all the other peaces of code (except comments, of course).

Conclusion
So how do you improve your MI metric? The first priority is to have less lines of code. The second priority is to reduce the amount of different operators and operands in a particular module. Both of these actions usually result in splitting up methods.

Again I would like to state that the MI metric is far from perfect and it should not be a definitive guide to what to refactor. It's only an indication of what to look at and I really think that it would be valuable to have comments taken into account, as well as have a slightly higher role for CC in there.

Please let me know your experiences on this subject trough the comments option below.

Exception causes Xml Error in deserialization

This week I seem to run into quite a few nooks and crannies in the .NET framework that put me in front of some tricky issues. This time it had to do with deserialization, or so Visual Studio led me to believe.

Here is the situation. Last week I wrote some code that deserialized an xml file into an elaborate object model. Part of that code was to parse an xsd file and add some objects to the model from there. This all worked well for the past week, as proven by the unittests.

Yesterday I started writing some code to dynamicly load assemblies and classes based on some information from the xsd file. I didn't get to finish it, but as I started working on it this morning, all of a sudden it threw an exception on the deserialization code, stating that I had some error in my xml file. I checked my xml file, which I knew I hadn't changed, and off course it was fine.

So I figured that since the last change I made was in some code that parsed part of the xsd file, I would debug there. As it appeared, the exception occurred whenever I passed a certain XPathNavigator instance into a method of my Factory class for creating a plugin. My first tought was that this occurred because I passed the XPathNavigator to another method. I started to program around this approach, but to no avail.

I crunched my head on it for another half hour until I had to leave for a meeting, but as I came back and looked at my code I realized I actually called the initializer method of a Factory class that I use to load some pluggable classes. I started debugging this method and presto, it raised an exception, because of some small oversight. I fixed this exception and the deserializer code worked again.

Conclusion on this is that some code in the XmlSerializer or it's base classes, catches any exception thrown and re throws another exception which is not related to the actual problem. Fortunately, the inner exception does contain the actual exception, meaning I could have saved myself a lot of time and headaches, by simple looking an the inner exception.

Another thing I learned was that using a singleton pattern to create your factory class is a good thing, but you must pay some attention when debugging.

Plugins and abstract static methods in C#

Here is what I ran into today. I'm currently working on this service that has to be expendable in some parts of the code. Because of the way things have to work, I figured I would use a technique based on plugins. I will look in a particular folder and load any assemblies available there. Then I will get all the types that derive from a certain base class and select the write plugin class based on some statically available information and then create an instance of this class...

...and that's where it all goes boom. Here is what I wanted to write as a base class:


public abstract class PluginBase
{
public abstract static string[] SupportedOperations { get; }

public abstract void ExecuteOperation(string operation);
}


I would then just loop trough the found plugin class types and check inside SupportedOperations to find out which plugin would be able to handle the task at hand. That won't work, because C# has no support for an abstract static member (so changing it into a method won't work either).

The reason for this, as I've read from several compiler designers at Microsoft, is that the compiler considers static members in derived classes as completely unrelated and it would be hard to resolve to the correct member and not very transparent to developers.

This also crushed any hopes of solving this with an interface, as an interface is implicitly abstract.

You can read a lot on the subject on these links:
http://msmvps.com/blogs/jon_skeet/archive/2008/08/29/lessons-learned-from-protocol-buffers-part-4-static-interfaces.aspx
http://blogs.msdn.com/ericlippert/archive/2007/06/21/calling-static-methods-on-type-variables-is-illegal-part-three.aspx
http://bytes.com/forum/thread238034.html
http://weblogs.asp.net/justin_rogers/articles/61042.aspx

I guess this means I'm not the only one that would like to use this as a feature (let me know, trough the comments).

If you read the comments on the first link, you may also run into this article, that actually shows that abstract static members are possible in IL. It's just not supported in C# (or any other well known .NET language for that matter).

So basically what is it that I want? I want a way to specify that any class derived from some base class has a specific static member that I can call. This can't be done, so I now know.

How did I solve it? I really didn't, but I went and made some design decisions. First of all I will add the static member to the base class, but not make it abstract, so my base class has an implementation of it's own. The base class implementation will return an empty string[], so I know it doesn't support anything (it's the abstract base class, what did you expect?).

Then how do you handle derived classes that don't have their own implementation of the static member? This depends on the use of the plugins. In my case, they are used in a service that has to be able to run unattended. For this reason I decided to apply defensive programming and simply ignore classes that don't implement the static member themselves.
In other cases it may make perfect sense to throw an expection, or handle it differently.

I hope this makes sense. If you have questions just let me know.

Using Source Control with Silverlight 2 and non-standard fonts

I just ran into this and tought I'd post a heads-up. When you build a Silverlight Class Library and use some fonts in it, that are not standard to Silverlight 2, the fonts you use are delivered to the browser plugin. To make it all work, it uses SubsetFontTask.dll. To make the fonts available it uses a .targets file which is imported into your project trough an tag.

This is all very nice and works as one might expect. Also its nice and transparent. If you don't look for it you probably don't even notice...

...until, that is, you add your project to source control. It appears that everything is just fine, until a collegue tries to get the same project working on his or her own computer. Then Visual Studio throws a nasty error about not being able to import the .targets file.

Fortunatly it's easy to fix. Just add the .targets file to source control. And then you simply do get latest version and...

...it breaks again, because it can't find the SubsetFontTask.dll in the root of the project where it expects it. So we add this file to source control as well. And then finally it works.

This doesn't seem right. I would expect that whenever I add a project to source control trough Visual Studio, it checks in all the files that are needed to build a project on any machine, that is properly installed.

Please let me know what you think about this, trough the comments below.

Unittesting and code coverage

Recently it was decided that all newly constructed code that our company builds, has to be unittested. Although this may seem like a late call, for a lot of product companies this is quite a big step and I, in contrast with a lot of developers out there, was happy to hear this...

...Until, that is, some mentioned that we should reach 100% code coverage. Now I know some of you out there would say that this should be the case, but I think differently. Let me explain.

Let's say you have some method that has to find an object in some hierarchy. At some point you end up writing some code like this:

FindableObject FindObject(string name)
{
FinadbleObject result = null;

foreach (FinadbleObject subObject in _subObjects)
{
if (subObject.Name == name)
{
result = subObject;
break;
}
else
{
result = subObject.FindObject(name);
if (result != null)
{
break;
}
}
}

return result;
}

It would work like a charm. Let's say you now write a unittest for this method. You set up a hierarchy of three objects and try to find the deepest one by calling FindObject on the top one. The result of the unittest would be a pass if the result of the method equals the initial object you placed there. Problem is, you would not reach 100% code coverage of your method, because the last } that belongs to the foreach loop would not be reached.

The only way to do this, is to write a unittest that will not render a result as it calles FindObject. Although this may very well be a valid unittest, I'd say that writing several lines of code, simply to test a closing curly bracket is somewhat over the top.

Another reason why I think having 100% code coverage in unittesting is highly impractical, is that you would have to test stuff like getters and setters on properties, even if they don't contain any code.

Please let me know if you think otherwise and have some compelling reason to still reach 100% code coverage, by dropping me a comment. I'm still open for debate on this.

Adding a customdictionary.xml to your project for use with FxCop

Today I was cleaning up some code I wrote, so I decided to run FxCop on my project to help me identify possible issues. The first warning I ran into was a "misspelled" word. In fact it was a term from our Business Layer which was in dutch, so I wanted FxCop to ignore it.

Reading the help topic on this warning pointed me in the direction of the CustomDictionary.xml file, which allows you to influence the way spelling is checked by FxCop. The help topic states that you can define one on installation level for the tool, on user level or for the project. In this case I decided it would be part of the project so I would work for the rest of our team as well.

So I simply added a CustomDictionary.xml to my project, looking like this:


<?xml version="1.0" encoding="utf-8" ?>
<Dictionary>
<Words>
<Recognized>
<Word>Translatie</Word>
</Recognized>
</Words>
</Dictionary>


Obviously MyWord replaces the actual word I needed to add, but you get the picture. So I ran FxCop again and my "misspelled" word still raised a warning!!

Googling on this subject got me to Duncan's Blog, who wrote this post about it. Simply changing the Build Action on the CustomDictionary.xml file (in the properties) to CodeAnalysisDictionary did the trick.


By the way, please tell me what you think about my blog, by posting a comment. Thanks.

Using XmlSerializer to serialize type information

During the development of a project I'm currently working on, I ran into this. One property of a class I defined is called SimpleType and it holds a Type object. It contains some basic type like int or string.
Part of the project is to store this information and have it easily configurable, so my first thought was to use the XmlSerializer class to simply serialize the objects inside a collection to an xml file. So I wrote some code to do this:


XmlWriter writer = XmlWriter.Create(@"c:\temp\template.xml");
XmlSerializer serializer = new XmlSerializer(typeof(Template));
serializer.Serialize(writer, template);
writer.Close();


Running this code threw a nice exception: "The type System.Int32 cannot be used in this context". At first I was stunned. I didn't have any properties that even related to int. Then I figured it must be the SimpleType property and I needed some kind of workaround for this behaviour.
Here is the code of the property I tried to serialize:


public Type SimpleType { get; set; }


To make it work I started by adding an XmlIgnore attribute to the SimpleType property. This obviously prevents the serializer from including it into the xml file and stops the error from occurring, but now I had no type information in my xml file and I did need it.
So I added a private field to store the type in and added a second property called SimpleTypeName of type string.
The getter of this property simply returns the SimpleTypes Name property. The setter uses the static Type.GetType method to translate a typename into a Type object and sets it in the field.
Here is the code after I rewrote it:


private Type _simpleType;

[XmlIgnore()]
public Type SimpleType
{
get
{
return _simpleType;
}
set
{
_simpleType = value;
}
}

public string SimpleTypeName
{
get
{
if (_simpleType != null)
{
return _simpleType.Name;
}
else
{
return string.Empty;
}
}
set
{
_simpleType = Type.GetType(value);
}
}


To make this work you obviously need the namespace System.Xml.Serialization.

If you have any questions or remarks, please feel free to comment below.

Using the popup element in Silverlight

It's been a while, as I was realy busy with getting our latest project of the ground, but finally here we are again.

This time, I wanted to look into the use of the popup element, as I plan on using this in future applications to handle some basic use cases. One of those is displaying a message to the user, for example an error message.

I started out with a simple application containing a grid as this is what most of my applications look like. As I wanted my popup to center in the browser window I added a canvas to the top left cell of the grid (in my case that would be the top row). Then I spanned the canvas trough all the rows and columns, so I covers the entire window. My xaml looks like this:


<grid name="LayoutRoot" background="White">
<grid.rowdefinitions>
<rowdefinition height="100">
<rowdefinition height="*">
<rowdefinition height="100">
</Grid.RowDefinitions>
<textblock row="0" margin="10" fontsize="20">Errorbox demo</textblock>
<button row="1" content="Throw error!" margin="20" click="Button_Click">
<textblock row="2" margin="10" verticalalignment="Bottom">Click the button to see a Popup control in action</textblock>
<canvas name="rootCanvas" row="0" rowspan="3">
</canvas>
</grid>

Next step is to actually build the popup for displaying error messages. It's as simple as wrapping any xaml you want for your popup inside a popup element. If you use Blend, like I do, what you may notice is that it doesn't display the popup by default. To have it displayed, you have to select the popup in the Objects and Timeline window. It then shows you the popup and it's contents.

The xaml for the popup looks like this:


<Popup x:Name="errorPopup">
<Border Background="#FFFFFFFF" BorderBrush="#FF000000" Height="300" Width="400">
<Grid x:Name="LayoutRoot" Background="White">
<Grid.RowDefinitions>
<RowDefinition Height="15" />
<RowDefinition Height="*" />
<RowDefinition Height="30" />
</Grid.RowDefinitions>
<Border Grid.Row="0" CornerRadius="15,15,0,0">
<TextBlock x:Name="TitleTextBlock"></TextBlock>
</Border>
<TextBlock Grid.Row="1" x:Name="ErrorText"></TextBlock>
<Border Grid.Row="2" CornerRadius="0,0,15,15">
<Button x:Name="OkButton" Width="100" HorizontalAlignment="Right" Margin="5" Content="Ok" Click="OkButton_Click"></Button>
</Border>
</Grid>
</Border>
</Popup>

So now we can write some code to popup our message. One thing that got me googling was how to actually show the popup. You use the IsOpen property which is a boolean. My code for displaying the popup looks like this:


public static void PopupError(Exception error, Canvas parent)
{
ErrorBox errorBox = new ErrorBox();
errorBox.Error = error;
parent.Children.Add(errorBox);
Canvas.SetLeft(errorBox, (parent.ActualWidth / 2) - errorBox.Width / 2);
Canvas.SetTop(errorBox, (parent.ActualHeight / 2) - errorBox.Height / 2);

errorBox.errorPopup.IsOpen = true;
}


When you want your popup to close, simply set the IsOpen property to false again. Note the use of the ActualHeight and ActualWidth. These are important because these are the dimensions that where set after rendering instead of the Height and Width property that are set in design mode. Another thing that you may run into is that these are set to 0. in the Loaded event of the page AND also in the Loaded event of the Canvas. If you rely on these events some redesinging is required to get this to work.

You can now download the project here. It's not all polished up, but you'll get the picture.

By the way, please let me now what you think about my blog and if there are any topics you would like to read about.