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

Recursively searching the Windows Registry

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

Problem

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)

Solution

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.

Code Snippets

On Error GoTo ErrHandler:
On Error GoTo ErrHandler
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
Dim reg As Registry
Dim reg As New Registry

Context

StackExchange Code Review Q#52122, answer score: 6

Revisions (0)

No revisions yet.