HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Declaration of bloatedness: the class that knew too much?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thedeclarationmuchknewtoothatbloatednessclass

Problem

I've already expressed concerns about this type's constructor, and I've sort-of* implemented the changes suggested in the answers I got there.

Recently I added even more parameters to that constructor - namely the ParentDeclaration and the Annotations. The Declaration type is absolutely where these things belong. For example when I come across this:
'@ignore VariableNotUsed
Dim foo As Integer


Then the Declaration object representing this foo variable contains an Annotation string that says @ignore VariableNotUsed, and the code inspections use that to know whether or not to raise a flag when References contains no IdentifierReference that isn't an assignment.

So I'm very much torn right now: I've always seen constructor-bloat as a violation of the Single Responsibility Principle... does this class break SRP?

In Rubberduck's API, a Declaration is just about anything: a VBAProject, a class or a standard (.bas) module, a form, the controls on a form, variables, constants, enums, enum members, procedures, functions - everything that has an identifier that can be referenced somewhere in code.

I need to clean up that constructor... it's fairly possible that I need to add more members in the future. It seems to me that everything has a purpose and is in its place. What am I not seeing?

Subclassing to chop off a parameter or two completely sounds like an ineffective way of misusing inheritance to solve the wrong problem. Does it?
Declaration.cs

```
namespace Rubberduck.Parsing.Symbols
{
///
/// Defines a declared identifier.
///
[DebuggerDisplay("({DeclarationType}) {Accessibility} {IdentifierName} As {AsTypeName} | {Selection}")]
public class Declaration
{
public Declaration(QualifiedMemberName qualifiedName, Declaration parentDeclaration, string parentScope,
string asTypeName, bool isSelfAssigned, bool isWithEvents,
Accessibility accessibility, DeclarationType declarationType, bool isBu

Solution

Implement IEquatable

Generally speaking, whenever I find myself writing overrides for object.Equals and object.GetHashCode, I also make the type implement IEquatable. It's generally pretty easy to do so by calling the IEquatable.Equals(T) method from the object.Equals(object) override:

public override bool Equals(object obj)
{
    return Equals(obj as Declaration);
}

public bool Equals(Declaration other)
{
    return other != null
        && other.IdentifierName == IdentifierName
        && other.ParentDeclaration == ParentDeclaration
        && other.Selection == Selection
        && other.Context == Context
        && other.Project == Project
        && other.QualifiedMemberName == QualifiedMemberName;
}

Code Snippets

public override bool Equals(object obj)
{
    return Equals(obj as Declaration);
}

public bool Equals(Declaration other)
{
    return other != null
        && other.IdentifierName == IdentifierName
        && other.ParentDeclaration == ParentDeclaration
        && other.Selection == Selection
        && other.Context == Context
        && other.Project == Project
        && other.QualifiedMemberName == QualifiedMemberName;
}

Context

StackExchange Code Review Q#106874, answer score: 10

Revisions (0)

No revisions yet.