debugModerate
Get Workbook Method(s)
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:
As always, general feedback is also welcome.
- 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 FunctionPublic 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 FunctionPublic 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 FunctionSolution
I will not mention again that I find the outdenting of
What error are you expecting to trap here?
This will raise runtime error 9 - Subscript out of range. By then you already know everything you need to know.
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:
Notice the difference between
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 0What 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 FunctionNotice 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 0Workbooks(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 FunctionContext
StackExchange Code Review Q#114156, answer score: 10
Revisions (0)
No revisions yet.