5
\$\begingroup\$

This function is part of a the Registry class that can be found on Google Drive. The class was originally written by Steve McMahon for VB6, but I ported it to VBA and added the function below.

I would like to know if I have made any mistakes in my logic and (if possible) how I could change this to keep searching after it has found the first instance.

Public Function findSectionKey(sectToFind As String, Optional sectToLookIn As String = vbNullString) As String
'*****************************************************************************
' Christopher J. McClellan 4-16-14
'
' Returns:
'   Full section key as string
'       ex: "software\wow6432Node\ODBC\ODBCINST.INI\Oracle in OraClient11g_home1"
'   If a matching section key is not found, returns an empty string.
'   Only returns first matching section key.
'
' Params:
'       sectToFind - string representing the keynode you're searching for.
'           ex: "ODBCINST.INI"
'       sectToLookIn - String representing the keynode to start the search in.
'           If omitted, use parent reg object's sectionKey value.
'*****************************************************************************
On Error GoTo ErrHandler:
Const PROC_NAME As String = "findSectionKey"

    Dim sSect() As String   ' string array of subnodes
    Dim iSectCount As Long  ' length of sSect array
    Dim reg As Registry     ' use a clone reg so we don't damage current object

    ' Test for optional sectToLookIn param
    If sectToLookIn = vbNullString Then
        sectToLookIn = Me.sectionKey
    End If
    ' create clone
    Set reg = New Registry
    With reg
        .ClassKey = Me.ClassKey
        .sectionKey = sectToLookIn
        ' create array of sections to search
        .EnumerateSections sSect, iSectCount
        ' search each section in array
        Dim i As Long
        For i = 1 To iSectCount
            'Debug.Print .sectionKey & "\" & sSect(i)
            If findSectionKey = vbNullString Then
                If sSect(i) = sectToFind Then
                    ' found node
                    findSectionKey = .sectionKey & "\" & sSect(i)
                    Exit For
                Else
                    'search subnodes via recursion
                    findSectionKey = findSectionKey(sectToFind, .sectionKey & "\" & sSect(i))
                End If
            Else
                Exit For
            End If
        Next i
    End With

ExitFunction:
    If Not (reg Is Nothing) Then
        Set reg = Nothing
    End If
    Exit Function
ErrHandler:
    'errBox TypeName(Me), PROC_NAME
    Resume ExitFunction
End Function
\$\endgroup\$
6
\$\begingroup\$

This is probably a typo:

On Error GoTo ErrHandler:

It should read as:

On Error GoTo ErrHandler

The : denotes a label, or separates multiple instructions on a single line, allowing you to write something like For i = 0 To 10 : DoSomething i : Next on a single line. Useful for the immediate pane, or for making your code harder to read by cramming multiple instructions on a single line.

I've seen this error-handling /exit pattern quite often, I find it's easier to follow/read the error-handling /exit pattern when the execution flow is more linear - the label could use a better name then:

Cleanup:

    'handle errors:
    If Err.Number <> 0 Then
        'errBox TypeName(Me), PROC_NAME
        Err.Clear
    End If

    'clean up regardless of error status:
    If Not (reg Is Nothing) Then
        Set reg = Nothing
    End If

    'clean exit
End Function

Doing this avoids resorting to jumps in the execution flow (Resume), and while it makes sense to say On Error GoTo Cleanup, it also makes sense to clean up in error cases as well, especially when dealing with external resources (a database connection, a registry key, a text file, etc.) - the only jump that needs to happen is a forward jump, only in case of an error: you want to skip everything else in the method's logic and jump straight to a clean exit.

If something goes wrong and I have a connected ADODB.Recordset object that blows up, I want to have code that closes the recordset and the connection (assuming the method opened them in the first place), and I don't want to copy that code into the happy path, ...and I don't want velociraptors to find me! To me the solution is simply to run that cleanup code whether or not an error is raised.


Dim reg As Registry

There's no reason not to New up this instance right away:

Dim reg As New Registry

That saves 1 line and a comment!


For i = 1 To iSectCount

Since the class doesn't specify Option Base 1 anywhere in the (declarations) section (just looked at the Google Drive link), it looks like there's a bug right here; you're systematically skipping index 0 - arrays are typically zero-based, and collections one-based:

MSDN (emphasis mine):

Because the default base is 0, the Option Base statement is never required. If used, the statement must appear in a module before any procedures. Option Base can appear only once in a module and must precede array declarations that include dimensions.

\$\endgroup\$
  • \$\begingroup\$ Thank you for pointing out the Option Base issue. I didn't write the original, so I'll have to really figure what's going on there. I'll respectfully disagree about the error handling pattern. I cringe every time I see someone intentionally allowing an error to occur. Personal preference. \$\endgroup\$ – RubberDuck May 31 '14 at 1:27
  • 1
    \$\begingroup\$ And now I finally understand why we always use for i = lbound(array). Thank you again. \$\endgroup\$ – RubberDuck May 31 '14 at 1:41
  • \$\begingroup\$ @ckuhn203 You're welcome! We need more vba and vb6 questions on this site, I hope you stick around! I've edited my answer to clarify on the error handling pattern. I'm not sure what you mean with allowing an error to occur though. \$\endgroup\$ – Mathieu Guindon May 31 '14 at 2:08
  • 2
    \$\begingroup\$ Oh no. I get what you're trying to do. It's more of a Try...Catch...Finally-ish control flow, but I'm solidly against that design in VB6/VBA. There's not a jump in control flow unless there's an error. In that case, there should be a jump in the flow. In an idea VBA world, you should always exit on Exit Sub and never reach End Sub. I'll be around. We can debate this later with competing answers to a new question. \$\endgroup\$ – RubberDuck May 31 '14 at 2:20
  • \$\begingroup\$ Dear future reader, please see this answer for a better explanation of my argument against Mat's error handling scheme. \$\endgroup\$ – RubberDuck Jun 2 '14 at 1:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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