debugMinor
FileWriter supporting writing to multiple files
Viewed 0 times
writingfilewriterfilesmultiplesupporting
Problem
I got bored with opening and closing files whenever I need to write to one, so I wrote a
If I change the
I don't need to close anything, but if I want to close an opened file, I can call
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:
The
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)
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 SubIf 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 ExplicitThe
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)
Solution
Using
The thing could be DRY'ed up a bit,
The function should be written to use
Removing items from
Now this is rather inefficient, because you already have a
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 FunctionThe 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 FunctionWriteLine 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 SubRemoving 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 IfNow 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.Code Snippets
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 FunctionPrivate 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 FunctionCatch:
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 SubCatch:
If Err.number <> 0 Then
result = False
CloseFile fileName
Err.Raise Err.number, Err.source, Err.description, Err.HelpFile, Err.HelpContext
End IfContext
StackExchange Code Review Q#52185, answer score: 6
Revisions (0)
No revisions yet.