# FileWriter supporting writing to multiple files

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

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


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

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.

Using Catch: as a label for On Error GoTo is a bad, misleading idea. Error handling in / 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.

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 .

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.

• 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 ;) – Mathieu Guindon Jun 1 '14 at 18:21
• 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. :) – David Harkness Jun 1 '14 at 18:28
• 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! – Mathieu Guindon Jun 1 '14 at 19:06
• 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! – David Harkness Jun 1 '14 at 19:34

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

• Hah, I like that! This is the first use of Choose I see and agree with! – Mathieu Guindon Sep 19 '17 at 17:42