Skip to main content
added 35 characters in body
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

Again, the procedure fits a handful of lines, and it's trivially easy to understand what's going on. There's a GetDestinationFilePath function that probably prompts for a folder and returns a full path/filename (using the provided info.Name), or an empty string if that prompt is cancelled by the user. It then proceeds to create some FileWriter object that is responsible for the file I/O, and the file is trivially written by invoking its Write method, given info.ToString, which presumably builds a string representation of the class module. The FileWriter class has a VB_PredeclaredId attribute set to True and exposes a Create factory method (disclaimer: I wrote that article) that takes the path/filename of the file to be created; presumably the Class_Terminate handler ensures the file handle is properly closed, but that's a low-level implementation detail that CreateClassModule doesn't need to be bothered with and, as a matter of fact, isn't.

Again, the procedure fits a handful of lines, and it's trivially easy to understand what's going on. There's a GetDestinationFilePath function that probably prompts for a folder and returns a full path/filename (using the provided info.Name), or an empty string if that prompt is cancelled by the user. It then proceeds to create some FileWriter object that is responsible for the file I/O, and the file is trivially written by invoking its Write method, given info.ToString, which presumably builds a string representation of the class module. The FileWriter class has a VB_PredeclaredId attribute set to True and exposes a Create factory method that takes the path/filename of the file to be created; presumably the Class_Terminate handler ensures the file handle is properly closed, but that's a low-level implementation detail that CreateClassModule doesn't need to be bothered with and, as a matter of fact, isn't.

Again, the procedure fits a handful of lines, and it's trivially easy to understand what's going on. There's a GetDestinationFilePath function that probably prompts for a folder and returns a full path/filename (using the provided info.Name), or an empty string if that prompt is cancelled by the user. It then proceeds to create some FileWriter object that is responsible for the file I/O, and the file is trivially written by invoking its Write method, given info.ToString, which presumably builds a string representation of the class module. The FileWriter class has a VB_PredeclaredId attribute set to True and exposes a Create factory method (disclaimer: I wrote that article) that takes the path/filename of the file to be created; presumably the Class_Terminate handler ensures the file handle is properly closed, but that's a low-level implementation detail that CreateClassModule doesn't need to be bothered with and, as a matter of fact, isn't.

Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

This post is tagged with , but what I'm seeing is very ironically procedural code.

It doesn't matter that this is "just a quick tool": we're here to write professional, quality code that is easy to read and maintain, performs well, and correctly. Comintern has given great feedback and highlighted a number of bugs and edge cases - which is exactly the purpose of this site and the reason one puts their code up for peer review, as opposed to just sharing it on GitHub.

Procedural code is essentially a sequence of executable statements. This main procedure is exactly that. If this were actual object-oriented code, it might have read like this:

Public Sub Main()

    Dim info As ClassInfo
    Set info = GetClassInfo(Selection)
    If info Is Nothing Then
        MsgBox "Invalid Selection. Review debug output for details."
        Exit Sub
    End If

    CreateClassModule info

End Sub

10 lines, including vertical whitespace. With proper abstraction, no procedure ever really needs to be much longer than that, and we know at a glance exactly what the procedure does, at a high level; if we need to look into the gory details of how a ClassInfo object gets created, we need to drill down to the GetClassInfo function, which we know will return Nothing if something goes wrong; if we need to look into the gory details of how a class module gets created, we need to navigate to the CreateClassModule procedure, which we know will take a ClassInfo parameter.

CreateClassModule might look like this:

Private Sub CreateClassModule(ByVal info As ClassInfo)

    Dim path As String
    path = GetDestinationFilePath(info.Name)
    If path = vbNullString Then Exit Sub

    With FileWriter.Create(path)
        .Write info.ToString
    End With

End Sub

Again, the procedure fits a handful of lines, and it's trivially easy to understand what's going on. There's a GetDestinationFilePath function that probably prompts for a folder and returns a full path/filename (using the provided info.Name), or an empty string if that prompt is cancelled by the user. It then proceeds to create some FileWriter object that is responsible for the file I/O, and the file is trivially written by invoking its Write method, given info.ToString, which presumably builds a string representation of the class module. The FileWriter class has a VB_PredeclaredId attribute set to True and exposes a Create factory method that takes the path/filename of the file to be created; presumably the Class_Terminate handler ensures the file handle is properly closed, but that's a low-level implementation detail that CreateClassModule doesn't need to be bothered with and, as a matter of fact, isn't.

So we need a definition for this ClassInfo object; we know we're going to need a ToString method and a Name property. Anything else? I can think of a number of things:

'@Folder("Tools.ClassBuilder")
'@ModuleDescription("Describes the metadata needed for generating a class module.")
Option Explicit
Private Type TClassInfo
    Name As String
    Description As String
    IsPredeclared As Boolean
    IsExposed As Boolean
    Members As Collection
End Type

Private this As TClassInfo

Private Sub Class_Initialize()
    Set this.Members = New Collection
End Sub

'@Description("Gets/sets the name of the class. Must be a valid identifier. Determines the value of the 'VB_Name' attribute.")
Public Property Get Name() As String
    Name = this.Name
End Property

Public Property Let Name(ByVal value As String)
    'TODO: validate input!
    this.Name = value
End Property

'@Description("Gets/sets the description of the class. Determines the value of the 'VB_Description' attribute.")
Public Property Get Description() As String
    Description = this.Description
End Property

Public Property Let Descrition(ByVal value As String)
    'TODO: validate input!
    this.Description = value
End Property

'@Description("Gets/sets the value of the 'VB_PredeclaredId' attribute.")
Public Property Get IsPredeclared() As Boolean
    IsPredeclared = this.IsPredeclared
End Property

Public Property Let IsPredeclared(ByVal value As Boolean)
    this.IsPredeclared = value
End Property

'@Description("Gets/sets the value of the 'VB_Exposed' and, indirectly, the 'VB_Creatable' attribute.")
Public Property Get IsExposed() As Boolean
    IsExposed = this.IsExposed
End Property

Public Property Let IsExposed(ByVal value as Boolean)
    this.IsExposed = value
End Property

'@Description("Adds the specified member metadata to this instance.")
Public Sub AddMember(ByVal info As MemberInfo)
    'TODO: validate input!
    this.Members.Add info, info.Key
End Sub

'@Description("Builds a string representing the entire contents of the class module.")
Public Function ToString() As String
    With New StringBuilder
        .AppendLine BuildHeaderInfo
        Dim member As MemberInfo
        For Each member In this.Members
            .AppendLine member.ToString
        Next
        ToString = .ToString
    End With
End Function

Private Function BuildHeaderInfo() As String
    With New StringBuilder
        .AppendLine "VERSION 1.0 CLASS"
        .AppendLine "BEGIN"
        .AppendLine "  MultiUse = -1  'True"
        .AppendLine "END"
        .AppendLine "Attribute VB_Name = """ & this.Name & """"
        .AppendLine "Attribute VB_GlobalNameSpace = False" ' no effect in VBA
        .AppendLine "Attribute VB_Creatable = " & CStr(Not this.IsExposed)
        .AppendLine "Attribute VB_PredeclaredId = " CStr(this.IsPredeclared)
        .AppendLine "Attribute VB_Exposed = " CStr(this.IsExposed)
        .AppendLine "Attribute VB_Description = """ & this.Description & """"
        .AppendLine "'@ModuleDescription(""" & this.Description & """)"
        .AppendLine "Option Explicit"
        BuildHeaderInfo = .ToString
    End With
End Function

Note the explicit ByVal modifiers and the absolute absence of any kind of Hungarianesque prefixing scheme.

The '@Annotation comments are picked up by Rubberduck (full disclosure: I am one of the administrators of this open-source VBIDE add-in project); they serve the dual purpose of documenting attribute values, and (through Rubberduck features) of enforcing these attribute values. Again note that the largest procedure here is a trivial series of .AppendLine calls on some StringBuilder object that's responsible for efficiently building a string, and again these are private implementation details of the ToString method, which does nothing more than append this file header info and each module members' own string representation to the result.

So there needs to be a MemberInfo class - that's essentially the role your clsGenClsMember class is playing. But your class is just data - an object encapsulates data, yes, but an object also performs operations on this data: from the code above we know a MemberInfo at least needs a ToString method, i.e. a way to turn its data into a string representation, and a Key property that gets a string that combines the member kind (Sub, Function, PropertyGet, PropertyLet, PropertySet) with the member's name, so that the keyed collection doesn't choke when a PropertyLet member is added for, say, a Name property when a PropertyGet member already exists for it.

You get the idea by now: the GetClassInfo procedure invoked in Main creates a ClassInfo instance, then trivially iterates the rows in the source Range to create MemberInfo instances and add them to the class metadata; if a property needs a getter and a setter, then two MemberInfo instances are added.

This isn't any more complicated than writing procedural code. In fact, I would quite vehemently argue that it's simpler - and much easier to debug/maintain, extend/enhance, and test. Not because it's "just a quick tool". Writing object-oriented code isn't especially hard; it's about how we think about code, about how we model the problem to be solved. IMO this "quick little tool" could be a perfect excuse to learn to write modern, object-oriented VBA code.