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

Get Workbook Method(s)

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

Problem

I'm re-writing my Module of standard methods. These 3 are used to retrieve Workbooks as Workbook Objects. The standard I'm aiming for here is "Third-party Library/Add-in". So:

  • Do these functions adhere to the principle of least surprise?



  • Have I missed any potential error cases?



As always, general feedback is also welcome.

Public Function GetWorkbook(ByVal strFileName As String, Optional ByVal strFilePath As Variant) As Workbook

        If Not (IsMissing(strFilePath) Or strFilePath.TypeName = "String") Then
            PrintErrorMessage "File Path was not supplied as a string", stopExecution:=True
        End If

    Dim wbIsOpen As Boolean

        wbIsOpen = IsWorkbookOpen(strFileName)
        If wbIsOpen Then
            Set GetWorkbook = Workbooks(strFileName)
        Else
            If Not IsMissing(strFilePath) Then
                Set GetWorkbook = OpenWorkbook(strFilePath & strFileName)
            Else
                PrintErrorMessage "Workbook (" & strFileName & ") is not open, and no filepath was supplied", stopExecution:=True
            End If
        End If

End Function


Public Function IsWorkbookOpen(ByVal strTargetName As String) As Boolean

    Dim wbTest As Workbook

        On Error Resume Next
            Set wbTest = Workbooks(strTargetName)
            IsWorkbookOpen = (wbTest.Name = strTargetName)
        On Error GoTo 0

End Function


Public Function OpenWorkbook(ByVal strFileName As String, strFilePath As String) As Workbook

    Dim strFullFilename As String
        strFullFilename = strFilePath & strFileName

    Dim wbOpenedSuccessfully As Boolean

        On Error Resume Next
            Set OpenWorkbook = Workbooks.Open(strFullFilename)
            wbOpenedSuccessfully = IsWorkbookOpen(strFileName)
        On Error GoTo 0

        If Not wbOpenedSuccessfully Then
            PrintErrorMessage "Failed to open workbook """ * strFullFilename & """", stopExecution:=True
        End If

End Function

Solution

I will not mention again that I find the outdenting of Dim statements hurt readability. Instead I'll focus on this part:

On Error Resume Next
    Set wbTest = Workbooks(strTargetName)
    IsWorkbookOpen = (wbTest.Name = strTargetName)
On Error GoTo 0


What error are you expecting to trap here?

Workbooks(strTargetName)


This will raise runtime error 9 - Subscript out of range. By then you already know everything you need to know.

(wbTest.Name = strTargetName)


This will raise runtime error 91 - Object variable or With block variable not set - but you already know if that call is going to work before you even try it.

This would be cleaner - slightly overkill with error-handling, but cleaner:

Public Function IsWorkbookOpen(ByVal wbFullFileName As String) As Boolean

    On Error GoTo CleanFail
    IsWorkbookOpen = Not Workbooks(wbFullFileName) Is Nothing

CleanExit:
    Exit Function
CleanFail:
    Err.Clear
    Resume CleanExit
End Function


Notice the difference between strTargetName and wbFullFileName: "str" says "that's a string". "wb" says "that's something about a workbook". Both are Hungarian, only one of them is doing it right. Avoid prefixes that indicate the type.

Code Snippets

On Error Resume Next
    Set wbTest = Workbooks(strTargetName)
    IsWorkbookOpen = (wbTest.Name = strTargetName)
On Error GoTo 0
Workbooks(strTargetName)
(wbTest.Name = strTargetName)
Public Function IsWorkbookOpen(ByVal wbFullFileName As String) As Boolean

    On Error GoTo CleanFail
    IsWorkbookOpen = Not Workbooks(wbFullFileName) Is Nothing

CleanExit:
    Exit Function
CleanFail:
    Err.Clear
    Resume CleanExit
End Function

Context

StackExchange Code Review Q#114156, answer score: 10

Revisions (0)

No revisions yet.