FileWriter supporting writing to multiple files

Question

I got bored with opening and closing files whenever I need to write to one, so I wrote a FileWriter class that's used like this:

Public Sub TestWriter()
 On Error GoTo ErrHandler
 Dim writer As New FileWriter
 If writer.OpenFile("c:\test.txt") Then
 writer.WriteLine "foo"
 End If
ErrHandler:
 If Err.Number <> 0 Then
 Debug.Print Err.Description
 End If
End Sub

---

If I change the writer.WriteLine "foo" call to writer.WriteLine "foo", "somefile.txt", I'll catch an error and output Invalid filename., as expected. I can also open both c:\test.txt and somefile.txt and then specify either file's filename when calling WriteLine, and write to either file.

I don't need to close anything, but if I want to close an opened file, I can call writer.CloseFile "c:\test.txt" and then I'll get an Invalid filename error if I try to write to it afterwards.

I don't need to explicitly anything, because any file the writer opens gets closed when the writer instance gets terminated / when the object goes out of scope.

Here's the code, I'm particularly interested in the error handling:

'expose raised errors to clients:
Public Enum FileWriterError
 InvalidFileName = vbObjectError + 42
End Enum
'manage opened files in a Dictionary:
Private openedFiles As Dictionary
'skip dictionary lookup if only 1 file is opened:
Private quickWriteFile As Long
Option Explicit

The Dictionary being used here is just a toy of mine, reviewable here. There are things I like less with that Dictionary (the fact that its Item getter gives me a KeyValuePair, for instance), but I'd like that to be outside the scope of this review.

Opening a file:

Public Property Get OpenedFilesCount() As Integer
 OpenedFilesCount = openedFiles.Count
End Property
Public Function OpenFile(ByVal fileName As String, Optional ByVal overwrite As Boolean = False) As Boolean
 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
 On Error GoTo Catch
 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)
Catch:
 If Err.number <> 0 Then
 Err.Clear
 End If
 OpenFile = (openedFiles.ContainsKey(fileName))
End Function

Writing a line of text into a file:

Public Sub WriteLine(ByVal data As String, Optional ByVal fileName As String = vbNullString)
 Dim fileNumber As Long
 Dim result As Boolean
 On Error GoTo Catch
 If CanWrite(fileName, fileNumber) Then
 Print #fileNumber, data
 result = True
 Else
 Err.Raise FileWriterError.InvalidFileName, TypeName(Me) & ".WriteLine", "Invalid filename."
 End If
Catch:
 If Err.number <> 0 Then
 result = False
 openedFiles.Remove fileNumber
 Err.Raise Err.number, Err.source, Err.description, Err.HelpFile, Err.HelpContext
 End If
End Sub
Private Function CanWrite(ByVal fileName As String, ByRef outFileNumber As Long) As Boolean
 Dim result As Boolean
 Dim fileNumber As Long
 Dim proceed As Boolean
 If quickWriteFile <> 0 And fileName = vbNullString Then
 fileNumber = quickWriteFile
 CanWrite = True
 Else
 CanWrite = openedFiles.TryGetValue(fileName, fileNumber)
 End If
 outFileNumber = fileNumber
End Function

Closing files:

Public Sub CloseFile(Optional ByVal fileName As String = vbNullString)
 If openedFiles.Count = 0 Then Exit Sub
 Dim fileNumber As Long
 fileNumber = GetFileNumber(fileName)
 If fileNumber <> FreeFile Then
 Close #fileNumber
 openedFiles.Remove fileNumber
 If fileNumber = quickWriteFile Then quickWriteFile = 0
 End If
 If openedFiles.Count = 1 Then quickWriteFile = openedFiles.Values.First
End Sub
Public Sub CloseAllFiles()
 Dim file As KeyValuePair
 For Each file In openedFiles
 Close #file.value
 Next
 openedFiles.Clear
 quickWriteFile = 0
End Sub
Private Function GetFileNumber(ByVal fileName As String) As Long
 Dim result As Long
 If quickWriteFile <> 0 And fileName = vbNullString Then
 result = quickWriteFile
 ElseIf Not openedFiles.TryGetValue(fileName, result) Then
 result = FreeFile
 End If
 GetFileNumber = result
End Function

Class initialization and termination:

Private Sub Class_Initialize()
 Set openedFiles = New Dictionary
End Sub
Private Sub Class_Terminate()
 CloseAllFiles
 Set openedFiles = Nothing
End Sub

Answer 1

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.

Additional Reading from Microsoft on VBA Error Handling

Answer 2

Using Catch: as a label for On Error GoTo is a bad, misleading idea. Error handling in vb6/vba has nothing to do with exception handling (try/catch). An error handling subroutine is typically best called ErrHandler.

The thing could be DRY'ed up a bit, CanWrite is getting a file number, and that functionality has already been extracted into GetFileNumber:

Private Function CanWrite(ByVal fileName As String, ByRef outFileNumber As Long) As Boolean
 Dim result As Boolean
 Dim fileNumber As Long
 Dim proceed As Boolean
 If quickWriteFile <> 0 And fileName = vbNullString Then
 fileNumber = quickWriteFile
 CanWrite = True
 Else
 CanWrite = openedFiles.TryGetValue(fileName, fileNumber)
 End If
 outFileNumber = fileNumber
End Function

The function should be written to use GetFileNumber instead of reimplementing the same logic:

Private Function CanWrite(ByVal fileName As String, ByRef outFileNumber As Long) As Boolean
 outFileNumber = GetFileNumber(fileName)
 If outFileNumber = FreeFile Then
 outFileNumber = 0
 Else
 CanWrite = True
 End If
End Function

---

WriteLine is removing the fileNumber from openedFiles when there's an error, but it's not clear that the file number in question refers to a closed file at that point, and then removing the file number from openedFiles will cause the file to remain opened when the class terminates, or even if client code explicitly calls CloseFile with the file name being used here.

Catch:
 If Err.number <> 0 Then
 result = False
 openedFiles.Remove fileNumber
 Err.Raise Err.number, Err.source, Err.description, Err.HelpFile, Err.HelpContext
 End If
End Sub

Removing items from openedFiles should be the job of the CloseFile method; if the error handler was written like this:

Catch:
 If Err.number <> 0 Then
 result = False
 CloseFile fileName
 Err.Raise Err.number, Err.source, Err.description, Err.HelpFile, Err.HelpContext
 End If

Now this is rather inefficient, because you already have a fileNumber, and CloseFile is going to get it again. Perhaps a private CloseFileNumber (or CloseInternal) method taking a file number instead of a file name could be useful here; CloseFile could then call this CloseInternal method after resolving the fileNumber from the given fileName.

Answer 3

Given that FileWriter works solely with files, you can drop File from the public method names leaving Open and Close, assuming these aren't reserved in vba.

It looks like the built-in Print automatically adds a newline unless you specify the new character position afterwards. If that's easy, e.g. without knowing how long the file currently is, it might be nice to add a Write method that does so.

• Mathieu Guindon: I actually intended to call them Open and Close, but they're indeed reserved keywords, so I added this File suffix - I agree it's redundant though, and I'll +1 for adding a Write method (WriteToFile? ...Write is reserved as well!) when votes reload ;)
• David Harkness: The best I can come up with so far is WriteText or WriteString. You need something to counter Line. WriteToFile would require changing the other to WriteLineToFile to maintain symmetry. :)
• Mathieu Guindon: How about WriteLine (with a line feed) and Append (without a line feed)? If withLineFeed Then Print #fileNumber, data Else Print #fileNumber, data; End If - the ; prevents writing the line feed, so I've implemented a WriteInternal private method that takes a withLineFeed Boolean, and WriteLine simply goes WriteInternal data, True, fileName while Append goes WriteInternal data, False, fileName. Pretty neat!
• David Harkness: That might imply that WriteLine starts from the beginning. How about Append and AppendLine? I know, WriteLine is so nice, and it's a FileWriter after all. Naming is hard!

Answer 4

In response to RubberDucks getErrorMessage procedure, I got the idea to use Choose (I was right now thinking about a possible use case of it) to make it looking more similar to the error enumeration and thus in my opinion easier to maintain:

Private Const ERROR_BASE As Long = &H100
Public Enum SystemError
 CustomErrorA = vbObjectError + ERROR_BASE
 CustomErrorB
 CustomErrorC
End Enum
Private Function GetErrorMessage(errorNumber As SystemError)
 GetErrorMessage = Choose(errorNumber - vbObjectError - ERROR_BASE + 1, _
 "Custom error A description", _
 "Custom error B description", _
 "Custom error C description" _
 )
End Function

• Mathieu Guindon: Hah, I like that! This is the first use of Choose I see and agree with!