-
Jan 31st, 2018, 11:24 AM
#1
Thread Starter
New Member
Structure and use of a class
I'm looking for somebody to take a quick look at this code I have for a class that reads a basic text file and inputs into my program. Currently you see it as it basically returns a message box with the results (which I'm testing a very small text file to confirm it works)
This first bit is from my "Main Form" that uses my class "Stream". (Stream was probably not the best name for it, but I'm just trying to understand it all at the moment.) The second bit is the actual class. My question for you experienced developers is, does this look like a good structure of a class and how I used it in the main form? I tried to keep each function small as possible (SRP) and tried to not repeat myself (DRY) I just finished reading "Pragmatic Programmer" and "Clean Code" and with my little knowledge I have, I'm trying to do things correctly or if that is too subjective, I want to do things that provide the flexibility when changes need to happen or other programmers need to look at my code. So please BE critical of this on any issues, suggestions, or good things you see. I really want to learn good habits and make sure I have a firm understanding. If there is books or anything of what I should learn please let me know.
Thanks!!
Code:
Public Class LogViewer
Dim str As String
Dim stream As New Stream(logFilePath)
stream.KeyWord = cboFilter.Text
stream.RemoveBlankLines = chkRemoveBlankLines.Checked
Str = stream.ReturnOrignalText
MsgBox(str)
End Class
Here is the code for the entire Stream class. I do have more things I would like to add to it.
Code:
Public Class Stream
Private fileStream As StreamReader
Private filterWord As String = String.Empty
Private removeBlankLine As Boolean
Private modifiedText As String
Private originalText As String
Private lineCount As Long = 1
Private line As String
Public Property KeyWord As String
Get
Return filterWord
End Get
Set(value As String)
filterWord = value
End Set
End Property
Public Property RemoveBlankLines As Boolean
Get
Return removeBlankLine
End Get
Set(value As Boolean)
removeBlankLine = value
End Set
End Property
Public Sub New(path As String)
fileStream = New StreamReader(File.OpenText(path).BaseStream)
End Sub
Public Function ReturnOrignalText() As String
Return ReadFile()
End Function
Public Function ReturnModifiedText() As String
Return ReadFileWithFilters()
End Function
Public Function ReadFile() As String
Do While Not fileStream.EndOfStream
line = fileStream.ReadLine
originalText &= line & vbNewLine
AddLineCount()
Loop
Return originalText
End Function
Public Function ReadFileWithFilters() As String
Do While Not fileStream.EndOfStream
line = fileStream.ReadLine
modifiedText &= ReturnModifiedLine()
AddLineCount()
Loop
Return modifiedText
End Function
Private Sub AddLineCount()
lineCount += 1
End Sub
Private Function NotEmpty() As Boolean
Return Trim(line) <> String.Empty
End Function
Private Function ReturnModifiedLine() As String
If NotEmpty() Then
Return ReturnFilterLine()
Else
Return ReturnBlankLine()
End If
End Function
Private Function ReturnFilterLine() As String
If line.Contains(filterWord) Then
Return line & vbNewLine
End If
Return Nothing
End Function
Private Function ReturnBlankLine() As String
If Not removeBlankLine Then
Return line & vbNewLine
End If
Return Nothing
End Function
End Class
-
Jan 31st, 2018, 12:24 PM
#2
Re: Structure and use of a class
Yeah, Stream is a particularly poor choice of a class name. "Streams" have been part of programming for several decades, by now, so even if you don't end up with a name collision for a class named Stream, it has too much baggage associated with it.
In general, it looks good. Those are good principles to follow them, and you followed them pretty well, though perhaps a bit TOO zealously. In particular, I noticed the AddCount method. The body of the function is only a single line. There is possibly some overhead for making the function call, in which case, that wouldn't be a good idea for just a single line of code. However, the (trivial) cost of the function call may be optimized away by the compiler, so it may make no difference at all. At first, I didn't see that you were calling the function from multiple places, but I still feel that a function that simple really adds nothing. After all, the function call is as big, and potentially slower, than the body of the function. I agree with the principle, I just feel that it was too zealously applied in that case.
Both ReturnOriginalText and ReturnModifiedText could be ReadOnly properties. That's a matter of taste, though. I doubt there's an advantage one way or the other.
The function NotEmpty is probably a bad idea. Any string manipulation, including Trim, is going to be a relatively costly operation. You won't see that cost in this case, unless you called that method thousands of times in a row, but it is still something to think about.
Aside from that, it really comes down to what you want to accomplish. If the code works, then it is fine. If it's an exercise in implementing a class that strictly adheres to certain principles, you've done that. However, it also isn't the most efficient class you could have written, and adhering to the principles may have made it a bit harder to see that. For example, ReturnBlankLines is going to return either "" (Nothing will end up as "" for a string), or it will return line & vbNewLine. However, when will it EVER return line & vbNewLine? It appears that the only time the method is called is in ReturnModifiedLine(), and then only if NotEmpty is False. Yet, when NotEmpty is false, then the line holds only "" or whitespace. In other words, ReturnBlankLines will only return either "" or vbNewLine, or in some rare cases <some invisible whitespace characters> and vbNewLine. Since the latter option amounts to vbNewLine, you really only return "" or vbNewLine from that method. That doesn't seem like the most efficient way to get to that end, but it has certainly split out the responsibilities pretty well.
My usual boring signature: Nothing
-
Jan 31st, 2018, 07:55 PM
#3
Thread Starter
New Member
Re: Structure and use of a class
Thanks Shaggy Hiker, this was very helpful and exactly the kind of input I was looking for. I feel better just knowing I'm on the right path and just need to keep at it. I do have one other question in regards to your comment that "Any string manipulation, including Trim, is going to be a relatively costly operation." How do I know if a particular method or current way of doing things is the most efficient or not. Like instead of using trim how would I know if there is a better way? Besides extensive testing is there like guide of comparisons?
-
Jan 31st, 2018, 08:23 PM
#4
Re: Structure and use of a class
Originally Posted by BikerCode
How do I know if a particular method or current way of doing things is the most efficient or not. Like instead of using trim how would I know if there is a better way? Besides extensive testing is there like guide of comparisons?
That only comes from research, testing and experience. There's never going to be an official guide because, on a micro scale, there's only one way to do each thing and, on a macro scale, there are just too many things include. Every persons needs will be slightly different so what's best for one is not necessarily best for others. Also, expensive operations are sometimes necessary but being able to identify them and best incorporate them, e.g. performing them in the background, is an important skill that you can improve over time.
-
Feb 1st, 2018, 07:53 AM
#5
Thread Starter
New Member
Re: Structure and use of a class
Thanks jmcilhinney, I appreciate the response and expertise and have definitely learned as a result!
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|