Before you start screaming "How is that better?", please read this answer of mine on Stack Overflow this answer of mine on Stack Overflow. The idea is to leverage the Microsoft Visual Basic for Applications Extensibility library to automate the maintenance of those specifically named constants prior to each build.
Before you start screaming "How is that better?", please read this answer of mine on Stack Overflow. The idea is to leverage the Microsoft Visual Basic for Applications Extensibility library to automate the maintenance of those specifically named constants prior to each build.
Before you start screaming "How is that better?", please read this answer of mine on Stack Overflow. The idea is to leverage the Microsoft Visual Basic for Applications Extensibility library to automate the maintenance of those specifically named constants prior to each build.
TL;DR Don't
Don't try to use Try...Catch...Finally block in VBA/VB6. The language isn't built that way. Your subs should always exit, not end.
Additional Reading from Microsoft on VBA Error Handling
TL;DR Don't try to use Try...Catch...Finally block in VBA/VB6. The language isn't built that way. Your subs should always exit, not end.
TL;DR
Don't try to use Try...Catch...Finally block in VBA/VB6. The language isn't built that way. Your subs should always exit, not end.
Additional Reading from Microsoft on VBA Error Handling
I'll start by saying that I like this. I like it a lot and I'll be doing this from now on.
'expose raised errors to clients:
Public Enum FileWriterError
InvalidFileName = vbObjectError + 42
End Enum
But, if you plan on raising more than one error, you should use a constant to give your class a range of error codes like this:
Private Const ErrorBaseNumber as Integer = 1000
'expose raised errors to clients:
Public Enum FileWriterError
InvalidFileName = vbObjectError + ErrorBaseNumber + 42
SomeOtherError
End Enum
Here SomeOtherError is implicitly set to vbObjectError + ErrorBaseNumber + 43. Ideally, you keep track of your base numbers somewhere and give all of your classes different base numbers.
I'd like to address your OpenFile() function a little bit. Where you've declared On Error GoTo Catch is a little late in your procedure. If for some (unlikely reason) you get an error before you've declared it, it won't be trapped. It should go directly after your declaration.
Public Function OpenFile(ByVal fileName As String, Optional ByVal overwrite As Boolean = False) As Boolean
On Error GoTo Catch
And do you really mean to Catch an error just to clear it? This is no better than using the On Error Resume Next statement. (Which should never be used. Never.)
You seem to be trying to implement a Try...Catch...Finally structure. That's not how it should be done in VBA/VB6. VBA/VB6 uses an "Always Exit" paradigm. In a perfect world, your code should never get to the End Function statement.
Here's a pretty standard pattern for error handling in VBA.
PublicFunction OpenFile(ByVal fileName as String, Optional ByVal overwrite as Boolean = False) As Boolean
On Error GoTo ErrHandler
Dim fileNumber As Long
fileNumber = GetFileNumber(fileName)
'guard against opening a file that's already opened:
If fileNumber <> FreeFile Then
OpenFile = True
Exit Function
End If
If overwrite Or Dir(fileName) = vbNullString Then
Open fileName For Output As #fileNumber
Else
Open fileName For Append As #fileNumber
End If
openedFiles.Add fileName, fileNumber
quickWriteFile = IIf(openedFiles.Count = 1, fileNumber, 0)
ExitFunction:
' clean up (if there was anything to clean up)
Exit Function
ErrHandler:
' Let's report the error to the user. Decide for yourself what to do with the error.
' Just do something with it.
MsgBox "Unexpected Error #" & Err.Number & " occurred." & vbCrLf & _
"Error Source: " & Err.Source & vbCrLf & _
"Error Procedure: OpenFile" & vbCrLF & _
"Error Description: " & Err.Description
Resume ExitFunction
End Function
In the Writeline Sub, I do like the way you raise errors, but it could lie to you if you for some reason change the name of the Sub, but forget to update the Function Name.
Err.Raise FileWriterError.InvalidFileName, TypeName(Me) & ".WriteLine", "Invalid filename."
Should be
Const ProcName As String = "WriteLine"
Err.Raise FileWriterError.InvalidFileName, TypeName(Me) & "." & ProcName, "Invalid FileName."
Before you start screaming "How is that better?", please read this answer of mine on Stack Overflow. The idea is to leverage the Microsoft Visual Basic for Applications Extensibility library to automate the maintenance of those specifically named constants prior to each build.
It would also be good practice to tie the "Invalid FileName" string to the FileWriterError.InvalidFileName enum via a private function.
Private Function getErrorMessage(ErrorNumber as FileWriterError)
Select Case ErrorNumber
Case InvalidFileName : getErrorMessage = "Invalid File Name."
Case SomeOtherError : getErrorMessage = "Some other error happened."
End Select
End Function
Blast me for the colon later, this is your code review, not mine =;)-
TL;DR Don't try to use Try...Catch...Finally block in VBA/VB6. The language isn't built that way. Your subs should always exit, not end.