HiveBrain v1.2.0
Get Started
← Back to all entries
debugModerate

File reader/writer (text)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
writertextfilereader

Problem

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
With file.CreateReader(path)
    data = .ReadToEnd
End With
Debug.Print data


Which is exactly what I wanted, and possibly as close to a using block that vb6 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
    Dim reader As FileReader
    Set reader = New FileReader
    If reader.OpenFile(path) Then Set CreateReader = reader
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.

FileReader Class Module

```
Private Const moduleErrorBase As Long = &HBEEF

Public Enum FileReaderError
FileNotOpened = vbObjectError + moduleErrorBase + 42
FileAlreadyOpened
End Enum

Private openedFileNumber As Integer
Private openedFileName As String

Private bofFlag As Boolean
Option Explicit

Public Function ReadToEnd() As String
Const method As Strin

Solution

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.

Readability

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

Code Snippets

CleanExit:
    OpenFile = True
    Exit Function

ErrHandler:
    OpenFile = False
    Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End Function
If Not bofFlag Then
    'file was partially read, output will be the remainder of the file
    'warn? raise error?
End If
Dim writer As FileWriter
Set writer = New FileWriter
With file.CreateWriter(path)
    .AppendLine "foo"
    .AppendLine "bar"
    .AppendLine "foobar"
End With
With file.CreateWriter(path)
    .AppendLine "foo"
    .AppendLine "bar"
    .AppendLine "foobar"
End With

Context

StackExchange Code Review Q#52306, answer score: 12

Revisions (0)

No revisions yet.