Jan Schreuder on .Net : Code inspections

What the f**k..

Jan Schreuder — Thu, 30 Oct 2008 12:10:00 GMT

I see a lot of code from other developers and every now and then you see some real gems. Today I was looking at a query to see if and how it could be optimised. One of the things I did was look at the query plan to see if indexes were used for various joins. One of the items in the plan made me look at a table definition, to check on it's indexes.

I then discovered that one of the tables joined into the query contained an index on every column?!?!?! I checked other queries that used the same table and discovered that all joins on that table were made on the column that made up the primary key. That primary key was a clustered index, so barely impossible to try and optimize that.

The F**K factor in this is of course what in heavens name posessed the developer that designed that table was actually thinking. The table contains 10 columns and 10 indexes. I removed all but the primary key and the query still performed as it was. Inserts and updates on the table are flying now, obviously.

Motto: Think before you index a column!!

How to - Create a missing designer.cs file

Jan Schreuder — Fri, 17 Oct 2008 09:49:00 GMT

I was running into this just now and found a quick way to solve it, so I just posted it for future reference.

I have an ASP.Net web application project in .Net 2.0 which was migrated from .Net 1.1 some time ago. I noticed that a few aspx pages did not have a designer.cs file. All controls on those pages were listed as protected variable in the main code behind page, like this example:

/// <summary>

/// An example label

/// </summary>

protected System.Web.UI.WebControls.Label Label2;

And it annoyed me. It was not consistent with the new pages in the application, and most other aspx pages that were also there during migration did have the designer.cs file. So how to solve this, in order to get a more consistent use of ASP.Net. I followed the following steps to do this:

First step - Make sure the latest version of the project is on your machine.
Second step is to remove the protected variables for the controls from the normal code behind cs file.
Now right-Click on the project and select 'Convert to Web Application'

The missing designer.cs is now created and added to the project. The protected variables, which were removed from the main code behind class are now in the partial class in the newly created designer.cs. The class in the original code behind class is also changed to a partial class.

If you forget step 2 and still want to remove the protected variables from your code behind class, then follow the following steps:

Remove the protected variables for the controls from the normal code behind cs file.
Open the ASPX file and make sure you view it in Source (HTML) mode
Select the entire HTML mark up using CTRL-A
Now press CTRL-K followed by CTRL-F

The HTML mark-up will be re-aligned and the designer.cs file will be recreated, including all the protected variables.

Syntax error? Really?!?!

Jan Schreuder — Thu, 25 Sep 2008 09:44:00 GMT

In my current project, we have adopted Microsoft StyleCop as a tool to make sure everyone sticks to the same style of coding. One of the things we currently incorporate in our daily work is making sure our existing code conforms to the rules we agreed on. Today however, StyleCop refused to check a class because of a syntax error in the following line of code:

double fraction = totalStaff > 0

    ? fraction = (double)completed / (double)totalStaff

    : fraction = 0;

The code you see here is part of a calculation to get the percentage of users that have completed a certain training module. The compiler sees no problem, and the result is as you would expect. But looking at the code, you do see something really weird. The variable fraction is always assigned twice. Maybe not a syntax problem, but strange and not necessary.

The following code does the same and only assigns the value once.

double fraction = totalStaff > 0

    ? (double)completed / (double)totalStaff

    : 0;

So why would someone do it twice?

Thank you, Lutz Roeder

Jan Schreuder — Tue, 26 Aug 2008 21:47:00 GMT

I just received an email from Lutz Roeder informing the users of Reflector that he decided to explore new opportunities. He has reached an agreement with RedGate software to continue work on Reflector. From his email:

I have reached an agreement to have Red Gate Software continue the development of .NET Reflector. Red Gate has a lot of experience creating development tools for both .NET and SQL Server. They have the resources necessary to work on new features, and Reflector fits nicely with other .NET tools the company offers.

Red Gate will continue to provide the free community version and is looking for your feedback and ideas for future versions.

For news and updates on Reflector, sign up for the .NET Developer’s Newsletter from Red Gate. To find out more about the agreement, see the interview on Simple Talk.

I really want to thank Lutz for all the great work he's done with Reflector, allowing us to explore the code inside .Net assemblies. He brought an excellent tool to our community and literally opened up .Net code for all to see

StyleCop - QuickStart tips

Jan Schreuder — Thu, 21 Aug 2008 07:15:00 GMT

After using StyleCop for some weeks, there are some things I think that might be useful to others when they want to start using the tool. So here are some tips.

Tip 1 - Also download the documentation

The documentation for StyleCop is available as a separate download. It explains why the rules are introduced and how you can fix violations. Without this documentation, you may think some of the rules are useless or annoying.

Tip 2 - Do not run the tool on an entire solution

At least, not untill you're satisfied that your code complies with the rules. You get so many warnings in a project or solution where the tool hasn't been used before, that you may think to stop using it immediately.

Tip 3 - Running the tool on one class file

Could have been in tip 2 as well, but I wanted to make a distinction here. When you right-click in the code, you will see the item Run StyleCop. Select this, and the tool will only check that particular class file. The number of warnings is signicantly less than when you run the tool on an entire project or solution, so the results are more obvious and motivate you more to keep using the tool.

Tip 4 - Disable rules you really disagree to

Rules you really don't want to check can be disabled using the StyleCopSettingsEditor. How this can be done is explained in this blog post. I disabled a number of them. I also included a number of overrides to the check for hungarian notation. For example, I have boolean variable names like isValid and isCompliant. StyleCop used to warn me about using hungarian notation in these cases, whereas I'm not. You can add the 'is' part in the Hungarian tab of the StyleCopSettingsEditor and it will no longer warn you about this.

Tip 5 - Re-Align the source before you start

Select all the code using Ctrl-A, then press Ctrl-K followed by F. Visual Studio will re-align much of the code. This automatically eliminates a number of errors found by StyleCop.

Version 4.3 of Microsoft StyleCop available

Jan Schreuder — Thu, 21 Aug 2008 06:45:00 GMT

A new version of StyleCop was made available only a few days ago, so I downloaded it and installed it. One thing I had to correct after installation was the Settings.SourceAnalysis file. Because of the name change to StyleCop, the extension of the settings file was also changed. But after that, I was back in business. One of the first things you notice is that the results are now shown as warnings in the standard errors and warnings list. And there are more changes:

A separate help file is available that explains the rules and shows how violations can be solved
A number of bug have been fixed
New rules, including a rule to check if Using directives are sorted, in line with the Visual Studio 2008 Organize Usings functionality

Another announcement on the StyleCop Teamblog suggested that they will be releasing SDK documentation to allow you to add your own rules to the tool. This is of course interesting if you have additional rules on top of the ones already available.

I've been using the tool for some weeks now and I really like the consistency in code style which is now slowly emerging in my projects. Where I thought I was using a consistent style in my own code, I was pointed out by StyleCop that in fact I wasn't. But that's all changing now.

You can download the new version of StyleCop here.

Have your C# coding style analysed by Microsoft, part 2

Jan Schreuder — Tue, 29 Jul 2008 13:21:00 GMT

Just as a small update on the use of StyleCop. I disagreed with some of the rules, like having to use spaces rather than tabs. It's not really an annoying rule, the only thing about it is that you have to agree on using spaces or tabs and then stick to it.

But still, I found out you can tune StyleCop to your needs. When you go the folder where the application is installed (on my machine C:\Program Files\Microsoft StyleCop 4.3) you will see a small application named StyleCopSettingsEditor.exe. Using that tool you can switch the rules supplied by the tool on or off. From a command prompt simply run the following command line:

StyleCopSettingsEditor.exe Settings.StyleCop

You will then see the following user interface:

Using this interface, you can simply select which rules are appropriate in your projects.

Coding standards, Coding rules, Coding guidelines, etc...

Jan Schreuder — Fri, 25 Jul 2008 20:11:00 GMT

If there's one thing that developers usually just cannot (or will not) agree to, then it's Coding standards/rules/guidelines or whatever name you want to give it. I posted a short item about a new tool from Microsoft that checks if your code matches the Microsoft guidelines and sure enough, I'm already getting mails on the subject explaining that Microsoft's rules suck and that they use one that's better. I thought it might just be a great subject for a discussion. But let me share my views on the subject first.

Coding standards, my view

I've been working as a developer in various roles and projects now for over 20 years. Yes, I'm that old, but it's to show that I speak from experience on this subject. Each project I worked on has had one or another form of coding guidelines or standards. In one project, they even called it a coding convention in an attempt to make it more important. My reaction in most occasions was: "What ever..."

The thing with this subject is, that developers simply cannot agree to one single guideline. Every developer thinks he/she is the best in his/her area and therefor the expert to tell others how to write their code. Well, they're wrong.

A coding standard is as good as the method to enforce it. If it's not enforced, then it's useless. And in 99% of all projects you will see that coding standards are not enforced. In my view, the only standard that works is the standard that you can enforce using tools. The guidelines checked by Microsoft's FxCop are a good example. It always uses the same rules for all code and always warns in the same way. There are other tools that do that as well, like DevPartner from Compuware. You can agree to those rules or not, but at least it leaves little room for discussion and I see FxCop being more and more accepted.

And now there's StyleCop. It checks your code against the coding rules as used at Microsoft development centers. Again, you can discuss if these rules are correct, but you get a tool that validates the code and tells you were you have strayed from the narrow path. My feeling is that although I don't entire agree to all rules, at least I can force my team to stick to these rules. Simply by breaking the build if they don't comply to the coding rules.

There are tools that allow you to write your own custom rules, like ReSharper and DevPartner. Useful as this can be, what do you do with code generated by Microsoft code generators. Think only of generated code for DataSet objects and WinForms. It's highly unlikely that this code will adhere to your guidelines. But will the use the Microsoft guidelines? It's a better bet then with your custom rules.

Another misconception is that these rules improve the quality. But to be frank, this is mostly bullshit. The rules checked by FxCop (and similar tools) check for code quality with respect to the use of the .Net framework. StyleCop (and similar tools) check for style issues. But that does not improve code quality. That can only be achieved by testing your work and using defense programming techniques. But I digress.

So whatever suits you, I guess. I'll stick to using the Microsoft Guidelines and use their tools to check if I've done it right. But whatever coding standard you choose, make sure your entire team sticks to those rules, otherwise they're completely useless. Unless it's to satisfy management that you thought about code quality.

But please, feel free to comment. I will approve them as soon as possible, but with the weekend starting in the Netherlands it might take some time. But I'd love to hear how the community feels about coding standards, the new Microsoft tool, and other things regarding this subject.

Have your C# coding style analysed by Microsoft!

Jan Schreuder — Fri, 25 Jul 2008 07:58:00 GMT

Or to be more specific, by Microsoft StyleCop. Now this may not be new to you, and that's very likely since StyleCop was announced in May of this year, but it was new to me.

The tool checks your code for coding style and more specific, the Microsoft coding style. It was developed outside the VSTS team and has absolutely no connections to FxCop. FxCop focusses on .Net Framework standards and correct use, StyleCop focuss on the way your code looks. Among others, it checks your code for the following items:

Layout of elements, statements, expressions, and query clauses
Placement of curly brackets, parenthesis, square brackets, etc
Spacing around keywords and operator symbols
Line spacing
Placement of method parameters within method declarations or method calls
Standard ordering of elements within a class
Formatting of documentation within element headers and file headers
Naming of elements, fields and variables
Use of the built-in types
Use of access modifiers
Allowed contents of files
Debugging text

The idea behind StyleCop, as can be read on their teamblog:

The ultimate goal of StyleCop is to allow you to produce elegant, consistent code that your team members and others who view your code will find highly readable. In order to accomplish this, StyleCop does not allow its rules to be very configurable. StyleCop takes a one-size-fits-all approach to code style, layout, and readability rules. It is highly likely that you will not agree with all of the rules and may even find some of the rules annoying at first!

I downloaded the tool (click here) and installed it to test drive it on my CrystalHelper class, and boy are they right. Some of those rules are annoying. I previously checked that class with FxCop and got little to comments there. From people that used the code, I always received comments that is was readable, easy to comprehend, etc. Then again, those differences make sense. FxCop looks at usage of the .Net Framework and does so by analyzing the compiled binaries. StyleCop uses the actual code. But let me give you some examples of what StyleCop has to say about my code.

Running StyleCop

The first thing I noticed when I ran StyleCop (or Source Analysis as it's called in the menu) is the enormous amount of comments. My first reaction was to remove the tool and forget about it. Actually, I had the same reaction when I first ran FxCop. I was curious enough to continue though, so let's look at what the tool had to say about the following code segment.

  489         /// <summary>
  490         /// Gets or sets the data source.
  491         /// </summary>
  492         /// <value>The data source.</value> 
  493         public DataSet DataSource 
  494         { 
  495             get 
  496             { 
  497                 return _reportDataSource; 
  498             } 
  499             set 
  500             { 
  501                 if (_reportDataSource != null) _reportDataSource.Dispose(); 
  502  
  503                 _reportDataSource = value; 
  504             }
  505         } 

Here we have 16 lines of code which most C# developers will accept as properly styled. StyleCop however gave me remarks about almost every line of code code:

Tabs are not allowed. Uses spaces instead. On almost every line.
The call to _reportData must begin with the 'this.' prefix to indicate that the item is a member of the class. On lines 497, 501, 502 and 504.
Statements or elements wrapped in curly brackets must be followed by a blank line. On line 498.
The body of the if statement must be wrapped in opening and closing curly brackets. On line 501.

In addition to this, it also mentioned that the fieldname should not start with an underscore. This was a remark abou the _dataSource class variable I use in this property code. For this section of code, I received a total of 23 comments!

Changing the code to satisfy the tool

As I said, most C# developers will not really have a problem with the coding style used in the above segment. But apparently, this is totally unacceptable for Microsoft developers. My next step was to change the above code so that the tool would not complain about this code. And this is what it looks like after those changes:

  489         /// <summary> 
  490         /// Gets or sets the data source. 
  491         /// </summary> 
  492         /// <value>The data source.</value> 
  493         public DataSet DataSource 
  494         { 
  495             get 
  496             { 
  497                 return this.ReportDataSource; 
  498             } 
  499  
  500             set 
  501             { 
  502                 if (this.ReportDataSource != null) 
  503                 { 
  504                     this.ReportDataSource.Dispose(); 
  505                 } 
  506  
  507                 this.ReportDataSource = value; 
  508             } 
  509         } 

All tabs are now replaced with spaces. The body if the if statement is now enclosed in brackets. There's an empty line after the closing brackets for the get body and the if body. All class members are now preceded by 'this.'. And yes, I removed the underscore from the class member. Not that much different, but some of the changes make sense, I guess.

Other comments found by the tool

The tool does find other things as well. Here's a short list of some other comments the tool made about my CrystalHelper class:

Invalid spacing around the comma. All comma's should be followed by a single space
Invalid spacing around the opening parenthesis. There should not be a single space after an opening parenthesis
Invalid spacing around the closing parenthesis. There should not be a single space before a closing parenthesis
The spacing around the symbol '=' is invalid. You need to add a single space before and after the '='symbol
All using directives must be placed inside of the namespace
All methods must be placed after all constructors
All methods must be placed after all properties
All private methods must be placed after all protected methods
All private methods must be placed after all public methods
All protected constructors must be placed after all public constructors
The method must have a documentation header
The summary section in the documentation header must not be empty

Should you use it?

A difficult question. My first reaction, as I said before, was to remove the tool and forget about it. But that was the same reaction I had when I first started using FxCop. And yet, that is now a tool I use on a daily basis.

I also have a problem with non-enforceable coding standards. Put 10 developers in a room to talk about coding standards and you end up with either 10 slightly different versions, or one that none of them will adhere to. My experience over the last 20 years is that, if you can't enforce coding standards, you can forget about introducing them and hope that it will all go well.

And that's exactly where this tool comes in. You can now enforce a number of rules to make code more readable and understandable. You might want to disagree to some of them (I don't agree to using spaces instead of tabs) but you can run the tool and check if your coding style matches that of the tool. And when all developers in your team use the tool and change their code accordingly, then all your code looks quite similar. Making it easier for your teammembers to work on code by other developers. And that's the main objective for any development team to create a coding standard.

The tool can also be included in MS-Build scripts, so you can easily make sure the build is broken when a developer ignores the coding standard. So yes, use it!

Some tips when using StyleCop

Once you have installed the tool and are ready to check your code using the tool, you might want to consider doing the following first:

Go to the C# section in Tools -> Options -> TextEditor and change the properties for the tabs so that it inserts spaces and click the OK button
Open the class file and select all code using CTRL-A
Now hold the CTRL key and press K, followed by F. This automatically re-aligns the entire class and replaces any tabs in your code with spaces. Do this for all class files in your project.

When you do this, the following will happen in your code.

Tabs are converted into spaces
Spacing before and after open and closing parenthesis is adjusted
Spacing around symbols is adjusted

StyleCop will not give yo any comments about those items. Which leaves the more interesting items to fix.

What's next for StyleCop?

Like all Microsoft tools, this is a work in progress. New features and options will be added and bugs will be fixed. But for the near future, the following has already been announced:

The tool is still called Source Analysis when you look for it in the Tools menu. It will be renamed to StyleCop
A small SDK will be made available to allow you to extend the tool with your own rules
Documentation about the currently available rules (which in fact opens up the Microsoft C# coding standards to the community)
Automatically change your code to enforce some of the rules

So keep an eye on the StyleCop blog for future releases of this tool. I will be discussing the tool with my team as soon as possible and start using it. Finally a way to have our code look similar!

What FxCop rules do Microsoft have turned on internally?

Jan Schreuder — Fri, 12 Oct 2007 07:06:00 GMT

Ever wondered if Microsoft use FxCop themselves, and which rules they have turned on? Well, the Visual Studio Code Analysis Team Blog gives you the answer. If you click this link, you will find a complete list of rules Microsoft use when analysing their code with FxCop.

FxCop 1.36 Beta available for download

Jan Schreuder — Thu, 11 Oct 2007 14:22:00 GMT

The Visual Studio Code Analysis Team Blog announces the release of FxCop 1.36 Beta.This new version contains the following (taken from the original post):

200+ bug fixes that reduce noise, missing analysis and rule crashes
Support for analyzing anonymous methods and lambda expressions
New option for skipping analysis over tool generated code
Better support for C++/CLI and the Compact Framework
Language 'friendly' API names in the UI and resolutions (ie Visual Basic syntax if running over a Visual Basic binary)
New globalization, design and usage rules
Performance improvements that cut analysis by 2x and use half as much memory
Documentation that is now available on MSDN

You can download it here!

Another "What the F**K" moment

Jan Schreuder — Tue, 02 Oct 2007 09:40:00 GMT

I tried compiling my application just now and got this error message from the compiler:

Preparing resources...

Updating references...

Performing main compilation...

error CS0583: Internal Compiler Error (0xc0000005 at address 77FCD43E): likely culprit is 'PARSE'.

An internal error has occurred in the compiler. To work around this problem, try simplifying or

changing the program near the locations listed below. Locations at the top of the list are closer

to the point at which the internal error occurred.

c:\inetpub\wwwroot\Intra\Hierarchy.aspx.cs: error CS0586: Internal Compiler Error: stage 'PARSE'

error CS0587: Internal Compiler Error: stage 'PARSE'

error CS0587: Internal Compiler Error: stage 'BEGIN'

Excuse me?!?!?! The last time I ever received errors like this was in 1996, while building applications for good-old MS-Dos. The Microsoft C compiler in some cases could not parse the code and presented you with a "Too complex" error. The only way around this was to re-order the code. But in the .Net age I never came across this error. Until now that is.

And what's even stranger is that a rebuild of the app solved the problem. I'm lost. Any one out there has any experience with this error?

What the F**K is the 'Grady' operator

Jan Schreuder — Mon, 01 Oct 2007 13:31:00 GMT

The web application I'm working on logs errors in a log file using Log4Net. Nothing new, loads of applications do that. But I was surprised when I found the following error message:

System.Data.SyntaxErrorException: Syntax error: Missing operand after 'grady' operator.

Now what in heavens name is a 'Grady' operator. I had no clue. Google only knows Grady from the book "Object-Oriented Analysis And Design With Applications" and other OO related work. So no solution to be found there. So what could it be?

I looked into the log file again and checked if the exception was raised more than once. I found more interesting 'operators': 'Toole', 'Amore','Sullivan'. Wait a minute there all names. I checked the stack traces for these exceptions and then it dawned on me. The error was raised when the user clicked a filter button. The web page where the error was raised showed a user list. Apparently, the user was looking for someone named O'Toole or O'Sullivan. I checked the code and found this:

if (txtUserFilter.Text != String.Empty)

    dataTable.DefaultView.RowFilter = string.Format("NAME like '%{0}%'", txtUserFilter.Text);

This will work fine of course, provided the text string does not contain any single quotes. The following solved my problem:

if(txtUserFilter.Text != String.Empty)

    dataTable.DefaultView.RowFilter = string.Format("NAME like '%{0}%'", txtUserFilter.Text.Replace("'", "''"));

Code inspections - Why and how to conduct them

Jan Schreuder — Tue, 10 Oct 2006 12:00:00 GMT

Triggered by a request from one of my colleagues, I decided to blog about why I think code inspections are useful and how they might be conducted. ...(read more)

Running FxCop on VB.net can be very annoying

Jan Schreuder — Thu, 31 Aug 2006 03:35:45 GMT

I built a very small application which only shows information from a table in a standard DataGrid. Nothing special, other than I had to do it in VB.Net 1.1. I have just finished testing it, and am now running FxCop on the code to see what problems I still have to fix before checking it into source safe.

The code contains a DataGridTextBoxColumn class which inherits from System.Windows.Forms.DataGridTextBoxColumn. Again, nothing particularly special. I override the Edit method to make sure no one gets to edit the line like this:

Protected Overloads Overrides Sub Edit(ByVal source As System.Windows.Forms.CurrencyManager, ByVal rowNum As Integer, ByVal bounds As System.Drawing.Rectangle, _

                ByVal isReadOnly As Boolean, ByVal instantText As String, ByVal cellIsVisible As Boolean)

    isReadOnly = True

End Sub

FxCop tells me the following about this method:

"Change parameter name 'isReadOnly' of method DataGridTextBoxColumn.Edit(CurrencyManager, Int32, Rectangle, Boolean, String, Boolean):Void to 'readOnly' in order to match the identifier as it has been declared in DataGridColumnStyle.Edit(CurrencyManager, Int32, Rectangle, Boolean, String, Boolean):Void."

Now I'm used to doing that in C#. No problem there. But when you do that in VB.Net, guess what:

C:\Projects\LogViewer\1.0\Helpers\DataGridTextBoxColumn.vb(22) :

         error BC30284: sub 'Edit' cannot be declared 'Overrides' because it does not override a sub in a base class.

C:\Projects\LogViewer\1.0\Helpers\DataGridTextBoxColumn.vb(23) :

         error BC30181: Specifiers valid only at the beginning of a declaration.

C:\Projects\LogViewer\1.0\Helpers\DataGridTextBoxColumn.vb(24) :

         error BC30203: Identifier expected.

Great huh, three compiler errors for the price of one! Makes sense of course, as ReadOnly is a VB.Net keyword to indicate a property is read-only and VB.Net is case insensitive. But it's behavior like this that can stop people from using FxCop. And that's a pity, because on other issues the remarks made by FxCop are valid.