# File reader/writer (text)

Following-up on this post, I wanted to be able to use my FileWriter with a syntax reminiscent of .net's using blocks, which ensure proper disposal of resources (i.e. closing the file/stream).

I'm looking for general feedback on the error handling and the overall cleanliness and readability of the code.

What I have can be successfully used like this:

Dim file As New file
Dim path As String
path = "c:\test.txt"

With file.CreateWriter(path)
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End With

Dim data As String
End With
Debug.Print data


Which is exactly what I wanted, and possibly as close to a using block that can get. This required implementing a small File class:

File Class Module

Option Explicit

Public Function CreateWriter(ByVal path As String, Optional ByVal overwrite As Boolean = True) As FileWriter
Dim writer As FileWriter
Set writer = New FileWriter
If writer.OpenFile(path, overwrite) Then Set CreateWriter = writer
End Function

Public Function CreateReader(ByVal path As String) As FileReader
End Function

Public Function Exists(ByVal path As String) As Boolean
Exists = (Dir(path) <> vbNullString)
End Function


I plan to eventually add members to this class, so that it becomes some kind of general-purpose "file helper"; the Exists method was added to that effect, and more will be added when I think of something else that would be useful to have there.

Private Const moduleErrorBase As Long = &HBEEF

FileNotOpened = vbObjectError + moduleErrorBase + 42
End Enum

Private openedFileNumber As Integer
Private openedFileName As String

Private bofFlag As Boolean
Option Explicit

Public Function ReadToEnd() As String
Const method As String = "ReadToEnd"
On Error GoTo ErrHandler

Dim result As String
Dim data As String

If Not bofFlag Then
'file was partially read, output will be the remainder of the file
'warn? raise error?
End If

While Not EOF(openedFileNumber)
Line Input #openedFileNumber, data
result = result & data & vbNewLine
Wend

CloseFile

CleanExit:
Exit Function

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Function

Public Function OpenFile(ByVal fileName As String) As Boolean
Const method As String = "OpenFile"
On Error GoTo ErrHandler

If openedFileNumber <> 0 Then OnFileAlreadyOpenedError method, fileName

openedFileNumber = FreeFile
openedFileName = fileName

Open openedFileName For Input As #openedFileNumber
bofFlag = True

OpenFile = True

CleanExit:
Exit Function

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description
End Function

Public Function ReadLine(ByRef data As String) As Boolean
Const method As String = "ReadLine"
On Error GoTo ErrHandler

If openedFileNumber = 0 Then OnFileNotOpenedError method

If EOF(openedFileNumber) Then
Exit Function
End If

Line Input #openedFileNumber, data
bofFlag = False

CleanExit:
Exit Function

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description
End Function

Public Sub CloseFile()
Const method As String = "CloseFile"
On Error GoTo ErrHandler

'If openedFileNumber = 0 Then OnFileNotOpenedError method, openedFileNumber

Close #openedFileNumber
openedFileNumber = 0

CleanExit:
Exit Sub

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Sub

Private Sub OnFileNotOpenedError(ByVal method As String)
Err.Raise FileWriterError.FileNotOpened, GetErrorSource(method), "File #" & openedFileNumber & "(" & openedFileName & ") was unexpectedly closed."
End Sub

Private Sub OnFileAlreadyOpenedError(ByVal method As String, ByVal fileName As String)
Err.Raise FileWriterError.FileAlreadyOpened, GetErrorSource(method), "File '" & fileName & "' cannot be opened with this instance at this point. A file is already opened."
End Sub

Private Function GetErrorSource(ByVal method As String) As String
GetErrorSource = TypeName(Me) & "." & method
End Function

Private Sub Class_Terminate()
CloseFile
End Sub


FileWriter Class Module

This code is the revised code from the previous/linked post, modified per reviews received. I have given up on supporting multiple opened files at once, since with the File class and the With syntax one gets much cleaner code, ..and it greatly cleaned up this code as well:

Private Const moduleErrorBase As Long = &HFADE

'expose raised errors to clients:
Public Enum FileWriterError
FileNotOpened = vbObjectError + moduleErrorBase + 42
End Enum

Private openedFileName As String
Private openedFileNumber As Long
Option Explicit

Public Function OpenFile(ByVal fileName As String, Optional ByVal overwrite As Boolean = True) As Boolean
Const method As String = "OpenFile"
On Error GoTo ErrHandler

If openedFileNumber <> 0 Then OnFileAlreadyOpenedError method, fileName
openedFileNumber = FreeFile
openedFileName = fileName

If overwrite Or Dir(fileName) = vbNullString Then
Open fileName For Output As #openedFileNumber
Else
Open fileName For Append As #openedFileNumber
End If

CleanExit:
OpenFile = True
Exit Function

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Function

Public Sub AppendLine(ByVal data As String)
AppendInternal data, True
End Sub

Public Sub Append(ByVal data As String)
AppendInternal data, False
End Sub

Private Sub AppendInternal(ByVal data As String, ByVal withLineFeed As Boolean)
Const method As String = "AppendInternal"
On Error GoTo ErrHandler

If openedFileNumber = 0 Then OnFileNotOpenedError method

If withLineFeed Then
Print #openedFileNumber, data
Else
Print #openedFileNumber, data;
End If

CleanExit:
Exit Sub

ErrHandler:

'handle "52: Bad file name or number" by raising a FileWriterError.FileNotOpened instead:
If Err.Number = 52 Then OnFileNotOpenedError method

'close file it *any* error occurs writing to it:
CloseFile

'bubble up all errors
Err.Raise Err.Number, Err.source, Err.Description
End Sub

Private Sub OnFileNotOpenedError(ByVal method As String)
Err.Raise FileWriterError.FileNotOpened, GetErrorSource(method), "File #" & openedFileNumber & "(" & openedFileName & ") was unexpectedly closed."
End Sub

Private Sub OnFileAlreadyOpenedError(ByVal method As String, ByVal fileName As String)
Err.Raise FileWriterError.FileAlreadyOpened, GetErrorSource(method), "File '" & fileName & "' cannot be opened with this instance at this point. A file is already opened."
End Sub

Public Sub CloseFile()
Const method As String = "CloseFile"
On Error GoTo ErrHandler

'If openedFileNumber = 0 Then OnFileNotOpenedError method, openedFileNumber

Close #openedFileNumber
openedFileNumber = 0

CleanExit:
Exit Sub

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Sub

Private Function GetErrorSource(ByVal method As String) As String
GetErrorSource = TypeName(Me) & "." & method
End Function

Private Sub Class_Terminate()
CloseFile
End Sub

• Just a readability preference, but your code window has a horizontal scroll bar. I would insert some _s. I know they are ugly but so is a 176 character line. – cheezsteak Jun 3 '14 at 18:28
• @ptwales that's a valid point! Don't hesitate to throw in an answer/review (and include that point) if there's anything else to add :) – Mathieu Guindon Jun 3 '14 at 18:52
• Again an old discussion digged out by me, but I need to mention that one should honestly pay attention to the Class_Terminate() events. – Unhandled Exception Sep 22 '17 at 11:20
• (sorry, lunch took too long, so I couldn't edit anymore) It es absolutely necessary to implement an error handler in there, because if any runtime error will be raised in this procedure, it is not possible to bubble it up! Instead VBA will stop execution. – Unhandled Exception Sep 22 '17 at 11:45

Error Handling

Your error handlers look much cleaner (and ultimately safer) than before. I also like your GetErrorSoure() routine and the "CleanExit:" name of the labels. Very concise. (<-Read as, "I'll be 'borrowing' more of your code'). I see a small issue in your OpenFile() routine. You should probably set OpenFile = False prior to re-raising the error. I know it's implicitly set to False, but it's good to leave it for the maintainer.

CleanExit:
OpenFile = True
Exit Function

ErrHandler:
OpenFile = False
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Function


That's extremely nit-picky though. They look good.

And this,

If Not bofFlag Then
'file was partially read, output will be the remainder of the file
'warn? raise error?
End If


go ahead and raise it. Let the coder decide what to do if it happens at a higher level.

I'm a fan of writing one line if statements, but you should probably bite the bullet and separate them out onto separate lines. Mr. Maintainer will thank you.

I'm conflicted about my next piece of advise. From a readability standpoint, it is correct to do this.

Dim writer As FileWriter
Set writer = New FileWriter


It is easier to read, but it's also an utter waste of effort because you immediately use the FileWriter. Given I don't like it because it's a waste of effort, there's no sense in changing it.

This is good. I like how you have the ability to use the with statement, but it feels weird to see it with a method. It's technically correct to use the verb-noun naming. It just feels weird. I keep wanting to just call it "Writer".

With file.CreateWriter(path)
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End With


Ok. I'm done nitpicking. It looks really good as far as I can tell. Just some food for thought now.

What if you wanted to extend this to allow random file io? How would you handle that?

I finally figured out how I would "fix" this.

With file.CreateWriter(path)
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End With


CreateWriter returns a FileWriter. For readability, I would want to call it like With writer, but this does mean using an extra variable.

Dim writer As FileWriter
Set writer = file.CreateWriter(path)

With writer
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End With

• Dim rw As New RandomReaderWriter? ...good question... – Mathieu Guindon Jun 3 '14 at 0:23
• You're right about Dim writer As New FileWriter, there's no reason why the Dim and the Set cannot be written as a single instruction. – Mathieu Guindon Jun 3 '14 at 4:15
• Yeah. The only reason not to is if you were going to declare it several lines before using it. – RubberDuck Jun 3 '14 at 10:53
• Blast from the past: As New is actually quite dangerous, as it changes how VB handles the object's lifetime: you can't set it to Nothing, for one - Rubberduck (the VBE add-in) issues a warning for that. – Mathieu Guindon Sep 22 '17 at 12:59

Error Handling:

• By ErrHandler you don't need to pass default args after Err.number and custom args:

ErrHandler:
Err.Raise Err.Number, GetErrorSource(method)


should be enough

• I like your GetErrorSource() method.

• I like your error numbers moduleErrorBase, enum etc.

Naming Conventions:

In VBA, it's nice to have vars, args, subs, functions, and methods with same style (a capital letter starting is the default) because VBA doesn't know different name cases. So if you have lowercase name, everything else with the same name will be lowercase too (very bad for version control).

For instance, someone using Writer() as property of some class. Your code rewrite the property name to lower case. You can use some prefix, to reduce the probability for name collisions, like Dim fwWriter and to improve readability

For your comment about "Dim and the Set as a single instruction":

There is reason to make 2 lines, which is its performance:

Never Dim anything As New if you're concerned about speed. VB6 will treat such a variable as an auto-instantiated variable. Every time you use it, VB will check if it should be instantiated. This will cost you some extra CPU cycles.

aivosto

• Actually if I wrote this today, the File class would not even need to be instantiated - I'd make it a "static class", with a VB_PredeclaredId attribute, making File.CreateReader usable directly. – Mathieu Guindon Jan 14 '15 at 11:58
• for more files funcs see access-codelib – ya_dimon Jan 14 '15 at 13:10
• @Mat'sMug IIRC this code closes connections when the class goes out of scope, so using a default instance would cause big bad file locking bugs. – RubberDuck Mar 1 '15 at 11:04
• @RubberDuck: Are you sure about this? In my opition, because the used variables for FileReader and FileWriter are in procedure scope only, they are out of scope after exiting the procedures and thus the resources are released immediately. Or am I wrong? – Unhandled Exception Sep 19 '17 at 14:39
• I have now checked this with Sysintenals 'Handle': The files will be released correctly. – Unhandled Exception Sep 19 '17 at 14:52