Oct 20, 2012 at 5:50 PM
Edited Oct 20, 2012 at 5:50 PM
When using EF to create a model for a database, the out-of-the-box generated code generates a number of FxCop warnings. For now I've worked around them by adding suppressions to the .tt templates in my project, but ideally the generated code would
be clean to start with.
Creating a simple model against the SQL 2012 AdventureWorks database generates the following warnings:
- CA1506: Warning for excessive class-coupling due to the fact there's a class referenced for each table in the database due to the one-to-one relationship of tables to types.
- CA2214: Warning for using virtual members in the constructor of each model where it contains at least one navigation property.
- CA2227: Warning for each navigation property as it is a collection property with a getter and a setter.
EF 5.0 generates some other warnings (CA1063), but these don't appear to be issues in EF 6.0.
I've submitted a pull request to suppress these messages, but it's been suggested that these may wish to be fixed instead by changing the generated code. Personally I think they should be suppressed as they appear to be design decisions made to improve
testability or are inherent to the EF approach to database modelling. My reasoning for each is as below.
CA1506 should be suppressed, as if you're working with a genuinely large database (50+) tables, then it would be overkill to pull the context apart into different context classes, as you'd end up with a fair amount of duplicate code, and may end up having
to use more than one context to perform operations between them, which would probably cause a lot of headaches.
CA2214 is fired due to the navigation properties being virtual. I'm guessing they were made virtual to allow tools like Moq to use setups on the navigation properties for use in unit tests. If this is the reason why they are designed as such,
it would seem to be safe to suppress the warning on the grounds that someone is unlikely to derive a class from a model type and override the properties manually in such a way that caused undesirable behaviour.
CA2227 is fired as the navigation properties are of type ICollection<T>, which causes FxCop to suggest that instead they should have a private setter to prevent them being set by public callers. Again, the current design appears to be sensible
to that the navigation properties can be easily accessed for setup in unit tests, otherwise you'd need to perform constructs such as:
Model foo = new Model();
foreach (var item in myMockCollection)
which is much more verbose that the current usage of:
Model foo = new Model();
foo.NavProperty = myMockCollection;
Based on the above, personally I think these three warnings should be suppressed so that the models that are generated are FxCop clean.
Any other thoughts on whether they should be suppressed or the design changed instead to eliminate the warnings in the first place?