I was recently asked to review the use of Stylecop as part of our standard procedures for ASP.Net projects. I'd not used it before, but was fairly happy to consider doing so as code styling has always been one of my pedantic points when reviewing code (including my own!).
Have to admit I was less keen once I'd installed it and found it complaining with around several hundred warnings - on a single, not that complex, and - I'd thought - beautifully coded class library.
Having uninstalled it, calmed down a bit a couple of weeks later, reinstalled it, and put aside an afternoon to work through one particular solution... came to the conclusion it did have quite a lot of value.
Although no individual change was of any great significance, the code certainly felt better once the refactoring effort had been completed - and we'd expect over time that any best practices it enforced we weren't adhering too, would become normal coding style.
So our plan is to use it on any greenfield projects, and to incorporate it into legacy projects on a case by case basis. The hope it it'll make working as teams of developers on a project easier and make picking up on a code base that someone hasn't worked on before more efficient.
Stylecop's settings if all left at the defaults are pretty strict... a bit too much so for my taste. Whilst keeping most in place, these were the few I felt reasonable to remove for our working practices.
First I removed a number of documentation ones. These fell into the category of agreeing with most of the time, but I didn't want it enforced 100% of the time. Were we in the habit of generated hard-copy documentation from the XML comments, I would leave them in place, but as we aren't our comments are purely for those reading the source code. In that case, I'd prefer comments were well-written and meaningful rather than completed purely for habit. For example, if a method is short, well-defined and sensibly named; it doesn't always need a comment.
Second to go was the requirement to list all methods in a class in accessibility order - i.e. all publics, then protected, then privates. Personally I'd prefer a local, helper private method to be close to where it is called from a public one, to aid code-readability.
Lastly, after some debate, we retained the option of prefixing with _ to indicate private variables. And related to this, didn't enforce the use of this to prefix local fields.
Some tips on refactoring
As mentioned, the plan is to use the tool mostly on greenfield projects. However at times we'll want to fix-up existing projects to adhere to the agreed styles. Having worked through a few now, these are the points I noted to at least slightly ease the pain.
- Probably use Resharper. I've not yet taken the time to install and learn this... but I probably should.
- Without that though, there are couple of other VS.Net extensions that I have used and can help with this. These are the Power Commands and the Productivity Tools. I actually removed the latter as it seemed to noticeably slow down by IDE, but the Power Command's Remove and Sort Usings is worth it by itself.
- Ctrl K + D will reformat code and fix some of the readability issues. No more "invalid spacing around the closing curly brace".
- The VS.Net menu Advanced > Untabify can be used to convert tabs to spaces on a file by file basis.
- Find/replace of course can fix up some.
- Lastly, when manually refactoring, I found it useful to order the list of warnings by file, and then within a file work from the bottom to the top. That way the line numbers remain correct as you double-click on each issue.