Should we start using FxCop and/or StyleCop in a mature project?

Question

We have 3 years old solution (.sln) with about 20 projects (.csproj). It is reasonable to start using FxCop and/or StyleCop? Maybe we should use it for several small projects first but not for whole solution?

It would be good to see some experienced answers.

Thanks.

EDIT: We are using TeamCity for continuous integration. And we have no possibility to use ReSharper. :( CodeRushXpress only.

Answer 1

Yes, you should, but slowly.

Get a copy of ReSharper, install StyleCop, and get the StyleCop for ReSharper plugin, setup which rules you'd like to use, and from then on every file you open will be full of wiggly blue lines to tell you where things are bad.

If you just fix them, one file at a time, you'll eventually end up with a nice clean project, without the need to convince your boss to let you spend 3 weeks going through your project doing nothing that results in chargable time!

Getting clean code is like refactoring, if you try and do it over an entire project all at once, you're going to end up in a pickle :)

Answer 2

Depending on how solution was created integration (and fixing issues reported by these tools) could take a lot of time, so I think integration of projects one by one is the way you should go.

EDIT:

In addition to Ed Woodcock's answer: you can configure SyleCop (and I bet FxCop too) to automatically run their checks during each VS build. Using this feature you can turn on checks on all of your projects one by one and fix all warnings (also you can configure these tools to generate errors) just during your regular development.

Answer 3

I've just started using StyleCop on my personal projects and it does take a little time to work through the "issues" raised.

I would recommend running StyleCop on a sample of your files and analysing the results before launching in to make any changes.

For example, by default StyleCop complains about missing method documentation for all methods, both public and private. Now, I can't answer this for you, but you need to decide whether you want private methods to have full documentation or not (there are arguments both ways). But you need to decide one way or the other. You don't want to get 6 months into making changes one way and then decide that you want it the other. That's either going to lead you to make unnecessary changes or have to revisit code you thought you'd finished.

Once you have make the necessary adjustments to StyleCop's settings and then let it loose on your code base - one project at a time.

Answer 4

Regarding FxCop, yes it is a good idea to make use of the tool, whether you are in a new project or an existing one. Much like StyleCop, you can run the tool and review the output. Unlike StyleCop, FxCop works on compiled code, not source.

It will probably be overwhelming at first. A good idea is to turn off all the rule groups, and rerun the tool to get a blank slate. Enable one group at a time, resolving any messages that appear, or turning off specific rules in that group if they do not apply to you (the default rules as a whole are quite broad, and not all will apply; you need to customize the ruleset for your needs).

At the end, you will have resolved all messages by either implementing appropriate fixes, or selectively disabling extraneous rules.

As a rule (no pun intended) I consider the Security and Performance groups good ones to start with. The Naming rules are subjective, and may clash with your own conventions. Turn them off if so. Mobility and Globalization are also subjective, and depend on your needs. As for the rest, well, you create your own conclusions!

Answer 5

An alternative or a good complement to FxCop/StyleCop would be to use NDepend and its Code Query language (CQL). CQL is dedicated to write code quality rules that can be verified live in Visual Studio, or that can be verified during build process and reported in a HTML report. Here are a few samples of CQL rules (designed to be highly customizable):

Code refactored recently should be 100% covered by test:

WARN IF Count > 0 IN SELECT METHODS WHERE CodeWasChanged AND PercentageCoverage < 100

Complex methods should be commented:

WARN IF Count > 0 IN SELECT METHODS WHERE CyclomaticComplexity > 15 AND PercentageComment < 10

I don’t want that my User Interface layer to depend directly on the DataBase layer:

WARN IF Count > 0 IN SELECT NAMESPACES WHERE IsDirectlyUsing "DataLayer" AND NameIs "UILayer"

Static fields should not be named m_XXX (Custom naming conventions):

WARN IF Count > 0 IN SELECT FIELDS WHERE NameLike "^m_" AND IsStatic

Methods out of MyAssembly and MyAssembly2 should not have more than 30 lines of code:

WARN IF Count > 0 IN SELECT METHODS OUT OF ASSEMBLIES "MyAssembly1", "MyAssembly2" WHERE NbLinesOfCode > 30

Public methods should not be removed to avoid API breaking changes:

WARN IF Count > 0 IN SELECT METHODS WHERE IsPublic AND IsInOlderBuild AND WasRemoved

Types tagged with the attribute MyNamespace.FullCoveredAttribute must be thoroughly covered by tests:

WARN IF Count > 0 IN SELECT TYPES WHERE HasAttribute "MyNamespace.FullCoveredAttribute" AND PercentageCoverage < 100

Answer 6

It depends:

• There is no value in changing code that is working for the sake of it.
• The StyleCop coding style make not match well with the style in your current code.
• You are likely to introduce bugs if you try to get your current code to pass FxCop and/or StyleCop
• A lot of the FxCop rules are of little or no value unless 3rd parties are writing code again your dlls.
• There is no point in using a checking tool if you ignore 101 warnings from it, as you will never spot the important one you care about.

I think it comes down to the possible reduction in the cost of maintain your current code base compared to the cost of getting the code base to pass the checking tools. A more consistent code base will have a lower cost of maintenance, but this is only of value if you are making lots of changes to the code base.