Rule S1200: Classes should not be coupled to too many other classes #548
Conversation
| KnownType.System_Single, | ||
| KnownType.System_Double, | ||
| KnownType.System_String, | ||
| KnownType.System_Object |
There was a problem hiding this comment.
Does FxCop count other special types, such as Func and Action? What about Task?
There was a problem hiding this comment.
Indeed, perhaps worth excluding all special types
There was a problem hiding this comment.
I made a test for fields and Task, Action and Func do count. I will update my tests to see if I correctly handle them (should be good). Any idea of other special types? Pointers don't count as they are of basic types.
| .SelectMany(node => ExtractTypeSymbolsFromSyntaxNode(node, model)); | ||
| } | ||
|
|
||
| private static IEnumerable<ITypeSymbol> ExtractTypeSymbolsFromSyntaxNode(SyntaxNode node, SemanticModel model) |
There was a problem hiding this comment.
perhaps if you use yield return in this method the code will look better: less created arrays, no need to return null in some cases (e.g. when parameter.Type == null), etc.
| KnownType.System_Single, | ||
| KnownType.System_Double, | ||
| KnownType.System_String, | ||
| KnownType.System_Object |
There was a problem hiding this comment.
Indeed, perhaps worth excluding all special types
| { | ||
| var classDeclaration = (ClassDeclarationSyntax)c.Node; | ||
|
|
||
| if (c.IsTest() || |
There was a problem hiding this comment.
Rspec doesn't say test are excluded, please update.
There was a problem hiding this comment.
Cannot update a sub-task on RSPEC.
There was a problem hiding this comment.
Then flag it to PMs and let them handle it.
| { | ||
| return parameter.Type != null | ||
| ? new[] { model.GetSymbolInfo(parameter.Type).Symbol as ITypeSymbol } | ||
| : new[] { (ITypeSymbol)null }; |
There was a problem hiding this comment.
I'd prefer default(ITypeSymbol), but it's not worth to change it now - more a comment for future.
| public abstract class TestCases // Noncompliant {{Split this class into smaller and more specialized ones to reduce its dependencies on other classes from 7 to the maximum authorized 1 or less.}} | ||
| // ^^^^^^^^^ | ||
| { | ||
| // ================================================================================ |
There was a problem hiding this comment.
That's a bit funny formatting :)
There was a problem hiding this comment.
It was a pain to read the different parts when I was doing my tests. I could actually remove the sections now if you guys want.
There was a problem hiding this comment.
Fine with me to leave it.
Fix #515