6
\$\begingroup\$

In order for Rubberduck to be able to "recognize" the built-in VBA functions, procedures and objects, I added yet another constructor parameter to my Declaration class:

public Declaration(QualifiedMemberName qualifiedName, string parentScope,
    string identifierName, string asTypeName, bool isSelfAssigned, bool isWithEvents,
    Accessibility accessibility, DeclarationType declarationType, ParserRuleContext context, Selection selection, bool isBuiltIn = false)
{
    _qualifiedName = qualifiedName;
    _parentScope = parentScope;
    _identifierName = identifierName;
    _asTypeName = asTypeName;
    _isSelfAssigned = isSelfAssigned;
    _isWithEvents = isWithEvents;
    _accessibility = accessibility;
    _declarationType = declarationType;
    _selection = selection;
    _context = context;
    _isBuiltIn = isBuiltIn;
}

That constructor is becoming a real mess, but now I can tell built-in declarations from the rest - this way I can know if the user is trying to rename the MsgBox function, and forbid it instead of happily letting the user break their code; as a bonus I can now also map identifier usages to these declarations, so the user can right-click any MsgBox function call, and I can show them everywhere they're using it in their VBA project.

So I proceeded to implement a little helper class that would give me an IEnumerable<Declaration> containing everything the VBA Standard Library exposes.

Because I'm only ever going to need to instantiate these declarations once, I decided to make it a static class, exposing nothing but a Declarations property getter.

At first I had a #region for each predefined module and class, and another for the predefined enums.. but I don't like #region, so I decided to use private nested types instead, and to define the Declaration instances as public static fields, so that I could easily use reflection to fetch all instances from all nested types, like this:

/// <summary>
/// Defines <see cref="Declaration"/> objects for the standard library.
/// </summary>
internal static class VbaStandardLib
{
    private static readonly QualifiedModuleName EmptyModuleName = new QualifiedModuleName();

    private static IEnumerable<Declaration> StandardLibDeclarations;

    public static IEnumerable<Declaration> Declarations
    {
        get
        {
            return StandardLibDeclarations ??
                   (StandardLibDeclarations = typeof (VbaStandardLib).GetNestedTypes(BindingFlags.NonPublic)
                       .SelectMany(t => t.GetFields().Select(f => (Declaration) f.GetValue(null))));
        }
    }

Here are a few of these nested types, just to illustrate what they look like:

private class VbaLib
{
    public static Declaration Vba = new Declaration(new QualifiedMemberName(EmptyModuleName, "VBA"), "VBA", "VBA", "VBA", true, false, Accessibility.Global, DeclarationType.Project, null, Selection.Home, true);

    public static Declaration FormShowConstants = new Declaration(new QualifiedMemberName(EmptyModuleName, "FormShowConstants"), "VBA", "FormShowConstants", "FormShowConstants", false, false, Accessibility.Global, DeclarationType.Enumeration, null, Selection.Home, true);
    public static Declaration VbModal = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbModal"), "VBA", "vbModal", "FormShowConstants", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbModeless = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbModeless"), "VBA", "vbModeless", "FormShowConstants", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);

    public static Declaration VbAppWinStyle = new Declaration(new QualifiedMemberName(EmptyModuleName, "VbAppWinStyle"), "VBA", "VbAppWinStyle", "VbAppWinStyle", false, false, Accessibility.Global, DeclarationType.Enumeration, null, Selection.Home, true);
    public static Declaration VbHide = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbHide"), "VBA", "vbHide", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbMaximizedFocus = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbMaximizedFocus"), "VBA", "vbMaximizedFocus", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbMinimizedFocus = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbMinimizedFocus"), "VBA", "vbMinimizedFocus", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbMinimizedNoFocus = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbMinimizedNoFocus"), "VBA", "vbMinimizedNoFocus", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbNormalFocus = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbNormalFocus"), "VBA", "vbNormalFocus", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    public static Declaration VbNormalNoFocus = new Declaration(new QualifiedMemberName(EmptyModuleName, "vbNormalNoFocus"), "VBA", "vbNormalNoFocus", "VbAppWinStyle", true, false, Accessibility.Global, DeclarationType.EnumerationMember, null, Selection.Home, true);
    ...

I've regrouped the members so as to mirror the structure of the VBA standard library, so the declarations contained in the VBA.Information module are defined in the InformationModule class:

private class InformationModule
{
    public static Declaration Information = new Declaration(new QualifiedMemberName(EmptyModuleName, "Information"), "VBA", "Information", "Information", false, false, Accessibility.Global, DeclarationType.Module, null, Selection.Home, true);
    public static Declaration Err = new Declaration(new QualifiedMemberName(EmptyModuleName, "Err"), "VBA.Information", "Err", "ErrObject", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IMEStatus = new Declaration(new QualifiedMemberName(EmptyModuleName, "IMEStatus"), "VBA.Information", "IMEStatus", "vbIMEStatus", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsArray = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsArray"), "VBA.Information", "IsArray", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsDate = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsDate"), "VBA.Information", "IsDate", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsEmpty = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsEmpty"), "VBA.Information", "IsEmpty", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsError = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsError"), "VBA.Information", "IsError", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsMissing = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsMissing"), "VBA.Information", "IsMissing", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsNull = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsNull"), "VBA.Information", "IsNull", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsNumeric = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsNumeric"), "VBA.Information", "IsNumeric", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration IsObject = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsObject"), "VBA.Information", "IsObject", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration QBColor = new Declaration(new QualifiedMemberName(EmptyModuleName, "QBColor"), "VBA.Information", "QBColor", "Long", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration RGB = new Declaration(new QualifiedMemberName(EmptyModuleName, "RGB"), "VBA.Information", "RGB", "Long", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration TypeName = new Declaration(new QualifiedMemberName(EmptyModuleName, "TypeName"), "VBA.Information", "TypeName", "String", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
    public static Declaration VarType = new Declaration(new QualifiedMemberName(EmptyModuleName, "VarType"), "VBA.Information", "vbVarType", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);
}

There are quite a few more, listing them here would be irrelevant.

Should I restructure this? This code isn't really going to be maintained, unless the VBA standard library suddenly earns new members... on the other hand, I might eventually decide to implement similar classes for commonly referenced VBA libraries, such as Scripting and ADODB - any thoughts?

\$\endgroup\$

2 Answers 2

Reset to default
4
\$\begingroup\$

I notice that there's a repeating pattern in all of your declarations...

The second parameter in QualifiedMemberName is always the same as the third parameter identifierName.

I'm assuming that QualifiedMemberName has a property for member/identifier so why not recycle that?

I think Heslacher's answer is the way to go. However, you could make life slightly easier for yourself by just creating a BuiltInMember class:

public class BuiltInMember : Declaration
{
    public BuiltInMemember (
            QualifiedMemberName qualifiedName, 
            string parentScope,
            string asTypeName,
            bool isSelfAssigned, 
            bool isWithEvents, 
            DeclarationType declarationType) 
        : base (
            qualifiedName, 
            parentScope, 
            qualifiedName.MemberName, 
            asTypeName, 
            isSelfAssigned, 
            isWithEvents,
            Accessibility.Global, 
            declarationType, 
            null, 
            Selection.Home, 
            true)
    {

    }
}

That gets you down to 6 constructor arguments in one step :)

\$\endgroup\$
4
\$\begingroup\$

More than 3 input parameters for a method is considered to be a code smell. One could say ok, I have 4 parameters but I can't help it, but 11 input parameters are way too many.

Problems I see with this type of object (Declaration) is, that the code gets error prone and prone to use a lot of copy and paste.

Instead of using a class you should consider to extract the properties and methods to an interface.
Then you should extract the different types of Declaration's by the meaning of the string asTypeName parameter to different classes which implements this interface.

Example:

Instead of 

public static Declaration IsNull = new Declaration(new QualifiedMemberName(EmptyModuleName, "IsNull"), "VBA.Information", "IsNull", "Boolean", false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);

you could use 

public static IDeclaration IsNull = new BooleanDeclaration(new QualifiedMemberName(EmptyModuleName, "IsNull"), "VBA.Information", "IsNull",  false, false, Accessibility.Global, DeclarationType.Function, null, Selection.Home, true);

where BooleanDeclaration would look like 

Public class BooleanDeclaration : IDeclaration
{
    public BooleanDeclaration(QualifiedMemberName qualifiedName, string parentScope,
        string identifierName, bool isSelfAssigned, bool isWithEvents,
        Accessibility accessibility, DeclarationType declarationType, ParserRuleContext context, Selection selection, bool isBuiltIn = false)
    {
        _qualifiedName = qualifiedName;
        _parentScope = parentScope;
        _asTypeName = "Boolean";
        _identifierName = identifierName;
        _isSelfAssigned = isSelfAssigned;
        _isWithEvents = isWithEvents;
        _accessibility = accessibility;
        _declarationType = declarationType;
        _selection = selection;
        _context = context;
        _isBuiltIn = isBuiltIn;
    }
}

As you can see the number of constructor parameters are reduced by one. The next to remove would be isWithEvents because a Boolean declaration won't ever be with events.

This can be done for all the basic types like Long, String etc.

By adding an overloaded constructor without the ParserRuleContext parameter you can reduce the number of parameters further.

Alos the Selection seems very often to be Selection.Home which should be reflected in another (or the same) overloaded constructor.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.