I ran FxCop against the Basternae source for the first time today. It ran 778,249 checks and found 10,850 issues.
While some of these really are design flaws, some minor and some serious, many of them are not applicable to this project, such as the security declarations and assembly signing. Here’s how I break down what FxCop has complained about:
* Array fields should not be read only.
* Avoid type names in parameters.
* Avoid uncalled private code.
* Avoid unnecessary string creation.
* Collections should implement generic interface.
* Consider passing base types as parameters.
* Do not cast unnecessarily.
* Do not concatenate strings inside loops.
* Do not declare visible instance fields. (This is what the encapsulation I’m working on is all about, and was a significant percentage of what FxCop complained about.)
* Do not initialize unnecessarily.
* Do not name enum values ‘Reserved’. (This was a surprise.)
* Do not pass types by reference.
* Do not raise reserved exception types.
* Enums should have a zero value.
* Flags enums should have plural names. (Dang, this one is pretty nitpicky.)
* ICollection implementations have strongly typed members.
* Identifiers should be cased correctly. (Yes, it actually complains about that, and there were a lot found.)
* Identifiers should have correct suffix.
* Identifiers should not have incorrect suffix.
* Identifiers should not match keywords. (Alias, Object, Event, and Exit classes.)
* Implement standard exception constructors.
* Interface methods should be callable by child types.
* Lists are strongly typed.
* Mark all non-serializable fields.
* Mark assemblies with assembly version (Been doing this, but missed one.)
* Mark ISerializable types with serializable. (Fixed these immediately. Duh.)
* Mark members as static. (It’s not the right solution for the ones FxCop found, but there are problems with what it found.)
* Nested types should not be visible.
* Non-constant fields should not be visible. (Encapsulation again.)
* Operations should not overflow.
* Operator overloads have named alternatives.
* Override equals and operator equals on value types.
* Provide correct arguments to formatting methods.
* Remove empty finalizers.
* Remove unused locals.
* Rethrow to preserve stack details.
* Review visible event handlers.
* Static holder types should not have constructors.
* Test for empty strings using string length.
* Type names should not match namespaces.
* Types that own disposable fields should be disposable.
* Use properties where appropriate.
* Validate arguments of public methods. (Note that FxCop does not appear to be Contract-aware, so some of these will be non-issues.)
Non-applicable issues found:
* Assemblies should declare minimum security.
* Assemblies should have valid strong names.
* Mark assemblies with CLSCompliant.
* Mark assemblies with ComVisible.
* Specify CultureInfo. (This is not an internationalized application.)
* Specifiy IFormatProvider. (Yet again, not an internationalized application.)
* Specify MessageBoxOptions. (This refers to right-to-left reading language support, also not applicable.)
Issues I don’t agree with:
* Do not catch general exception types. (Sometimes it really is the simplest and best solution.)
* Do not declare read only mutable reference types.
* Do not expose generic lists.
* Do not pass literals as localized parameters. Use a resource table instead.
* Identifiers should be spelled correctly. (FxCop didn’t like my use of “x” and “y” as variables in a coordinate class.)
* Identifiers should not contain underscores. (There are a ton of these, about 30% of the Cop’s finds. Pretty much all of our flags are named using underscores.)
* Long acronyms should be Pascal-cased. (Side effects of the previous issue.)
* Only FlagsAttribute enums should have plural names. (Stop whining about names already!)
* Prefer jagged arrays over multidimensional. (No. There’s a specific reason for multidimensional arrays.)
* Short acronyms should be uppercase.
* Used preferred terms.
So yes, there are quite a few things to be fixed. I’m not going to spend any extra effort on that right now — some of these problems will resolve themselves as part of the in-progress changes. Even so, it’s nice to be aware of what doesn’t match preferred style, even if it accomplishes nothing other than having a solid knowledge of which rules are being broken on purpose. I’m sure I’ll post updates on FxCop stats as the code evolves.