Skip to content

einarwh/StyleCopRules

Folders and files

NameName
Last commit message
Last commit date

Latest commit

 

History

34 Commits
 
 
 
 
 
 

Repository files navigation

StyleCopRules

StyleCop settings intended to help rather than harm your project.

Introduction

Used correctly, StyleCop is a useful tool for enforcing healthy code hygiene in your C# project. Used incorrectly, it can cause considerable harm by prompting developers to resort to bad practices like autogenerating documentation or wasting the developers' time by forcing them to manually keep their code elements sorted.

StyleCop should be gentle and almost invisible rather than intrusive. It should get out of the developers' way, yet keep them from writing sloppy code by accident. Therefore, there is little room for rules that generate false positives. In general, a candidate rule needs to provide benefits that significantly offset the cost for developers in order to be included in the analysis. When in doubt, it is better to leave a rule out.

This project contains my subjective assessment of the available StyleCop rules, as well as a settings file that corresponds to the assessment.

Disclaimer

Hopefully this is obvious, but still: I wish to make it very clear that I appreciate the effort put into making StyleCop, which is a useful tool that aids quality-minded .NET developers all over the world every day. While I make some opinionated assessments of some of the rules below, it doesn't mean I think poorly of the good people behind the tool. Quite contrary, I wouldn't be making this assessment if I didn't think StyleCop is an excellent tool worth spending time on - I just want to maximize the benefit I get from it. Moreover, this is obviously a single developer's opinions, based on subjective experiences and the specific contexts I've worked in. Your milage may and will vary.

Assessment of rules

Documentation Rules (SA1600-)

I think it's a very bad idea to use a static analysis tool to mandate documentation of all code elements. It inevitably leads to very low-quality documentation. The only reason I can think for doing so is that it leads to high documentation coverage. However, measuring documentation coverage is a completely meaningless exercise because it is easy - trivial - to autogenerate documentation (using ReSharper, GhostDoc or what have you). Autogenerated documentation is worthless. In fact, it's even worse than that: it has negative value. Autogenerated documentation is a source of noise that leads to code that is less readable and makes it harder to find genuine documentation in the code base.

See http://www.stylecop.com/docs/Documentation%20Rules.html

Sane

Insane

Layout Rules (SA1500-)

I think it's OK to follow some layout rules, to the extent they help the project avoiding code that is simply sloppily written.

See http://www.stylecop.com/docs/Layout%20Rules.html

Sane

Insane

Maintainability Rules (SA1400-)

To claim that these rules vastly improves maintainability is probably stretching it, but they don't put any heavy burdens on the developer either. The false positive ratio will probably be very low.

See http://www.stylecop.com/docs/Maintainability%20Rules.html

Sane

Naming Rules (SA1300-)

It pretty limited what a purely syntactic analysis tell us about the quality of names.

See http://www.stylecop.com/docs/Naming%20Rules.html

Sane

  • SA1300: ElementMustBeginWithUpperCaseLetter

    Fair enough, it's the convention.

  • SA1302: InterfaceNamesMustBeginWithI

    It's actually a terribly silly rule, but unfortunately it's always been the convention in .NET code that all interfaces should begin with an I. This is the official recommendation from Microsoft. The naming guidelines tells us to "DO prefix interface names with the letter I, to indicate that the type is an interface.". At the same time, we are told that we should avoid Hungarian notation. It makes no sense. The recommendation probably stems from the misunderstanding that interfaces and classes always go in pairs: one implementing class per interface. Then there's an assumed conflict between the interface and the class as to which one gets the "real" name and which one gets a derivation. According to such a warped view, the choice is between IFoo (interface) and Foo (class) and Foo (interface) and CFoo or FooImpl (class). A much better solution is to solve this contention by having the class name reflect some concrete aspect of the implementation. An example would be to have a List interface with classes LinkedList and ArrayList. But alas, this battle is lost and there is no point in fighting wind-mills.

  • SA1304: NonPrivateReadonlyFieldsMustBeginWithUpperCaseLetter

    Uncertain. I suppose the rationale is that non-private fields should have the same syntactic form as properties.

  • SA1307: AccessibleFieldsMustBeginWithUpperCaseLetter

    Ref SA1304.

Insane

Ordering Rules (SA1200-)

In general, I don't think it makes sense to spend the developers' time sorting code elements according to arbitrary criteria like access modifiers or the alphabet. It creates unnecessary work without improving code navigation.

See http://www.stylecop.com/docs/Ordering%20Rules.html

Sane

Insane

Readability Rules (SA1100-)

As always, StyleCop is mainly helpful when assisting in maintaining elementary code hygiene and consistency of formatting. As corrective-aggressive monkey (ref SA1122), StyleCop is annoying and outright damaging.

See http://www.stylecop.com/docs/Readability%20Rules.html

Sane

Insane

  • SA1101: PrefixLocalCallsWithThis

    No chance.

  • SA1121: UseBuiltInTypeAlias

    I'm inclined to think that it is better to use the real type names when referring to static methods defined by those types, e.g. String.Compare("foo", "bar") is better than string.Compare("foo", "bar").

  • SA1122: UseStringEmptyForEmptyStrings

    There is an established best practice in the .NET world that you are supposed to use string.Empty rather than "", but I think it's ridiculous. IMHO "" is preferable, since it is both more concise and more readable. There is also some superstition out there that string.Empty is supposed to be more performant than "", but that's pure hogwash. Since .NET 2.0, there has been no difference in performance between the two alternatives (and besides: talk about micro optimization!). Developers who let this myth dictate how they write code today should google "the monkey and the ladder". For a more thorough discussion, see http://stackoverflow.com/a/263257.

  • SA1124: DoNotUseRegions

    I don't like regions (refactor if things get big and unwieldy!), but there might be cases where regions are OK and the overall principle is to err on the lenient side when in doubt.

  • SA1126: PrefixCallsCorrectly

    WTF? No.

Spacing Rules (SA1000-)

This is primarily about simple code hygiene. This is what StyleCop does best.

See http://www.stylecop.com/docs/Spacing%20Rules.html

Sane

Insane

About

StyleCop settings intended to help rather than harm your project.

Resources

License

Stars

Watchers

Forks

Releases

No releases published

Packages

 
 
 

Contributors