debugModerate
File reader/writer (text)
Viewed 0 times
writertextfilereader
Problem
Following-up on this post, I wanted to be able to use my
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:
Which is exactly what I wanted, and possibly as close to a
File Class Module
I plan to eventually add members to this class, so that it becomes some kind of general-purpose "file helper"; the
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
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 dataWhich 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 FunctionI 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.
That's extremely nit-picky though. They look good.
And this,
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.
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".
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.
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 FunctionThat'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 Ifgo 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 FileWriterIt 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 WithOk. 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 WithCreateWriter 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 WithCode Snippets
CleanExit:
OpenFile = True
Exit Function
ErrHandler:
OpenFile = False
Err.Raise Err.Number, GetErrorSource(method), Err.Description, Err.HelpFile, Err.HelpContext
End FunctionIf Not bofFlag Then
'file was partially read, output will be the remainder of the file
'warn? raise error?
End IfDim writer As FileWriter
Set writer = New FileWriterWith file.CreateWriter(path)
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End WithWith file.CreateWriter(path)
.AppendLine "foo"
.AppendLine "bar"
.AppendLine "foobar"
End WithContext
StackExchange Code Review Q#52306, answer score: 12
Revisions (0)
No revisions yet.