Efficient Programming Help - FileOpen

NeuralJack · Aug 2, 2006

I'm wondering if there is a specific forum sub-topic that I should post various questions I might have on simple efficiency. We all know that there are tons of ways to program a certain function.. i'd like to talk, now and then, about how efficient various methods of getting the job done are. Also, whether or not that method is best for vb.net or is it old-school VB programming that should be updated.
One wouldnt really use the sub-topics, I wouldnt think, because it's not a question of implementation but more of a question of efficiency or using the best functions to get the job done. Often, the simplicity of the solution is something to consider too. If it takes 1 line to do something old-school method as opposed to 10 lines of VB.net code, that's something to factor in.

Also, is there a better VB.Net forum out there? From what i've seen this place is the best and I plan on becoming much more active here.

My question on efficient programming today is that I still use the FileOpen Function to Save/Load my settings files for my programs. It's simple and effective. Any suggestions for updating this?
I also use a while loop to read the variables it's loading from the file so that the order in which the lines are read from the file do not matter nearly as much.

Here is how I currently code loading a settings file:

Code:

    Public Sub LoadSettingsFromFile(ByVal FileName As String)
        On Error GoTo ErrorHandler
        Dim fso As New Scripting.FileSystemObject()
        If Not (FileName.equals(""))Then
            If fso.FileExists(FileName) Then
                Dim tmpString As String
                Dim QuitReadingFile As Boolean = False
                FileOpen(1, FileName, OpenMode.Input)

                While QuitReadingFile = False And Not (EOF(1))
                    Input(1, tmpString)
                    If InStr(tmpString, "DONE") Then
                        QuitReadingFile = True
                    ElseIf InStr(tmpString, "Variable1=") Then
                        var1 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                    ElseIf InStr(tmpString, "Variable2=") Then
                        var2 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                    End If
                End While
                FileClose(1)
            Else
                MsgBox("Can not find the file containing the variable." + vbCrLf + _
                "The Specified File = " + FileLocation_GameWindowTitle1)
            End If

        End If
        Exit Sub
ErrorHandler:
        MsgBox("An Error Occured in LoadSettingsFile SubRoutine." _
+ vbCrLf + "Error Number = " + Err.Number.ToString + vbCrLf +_
 "Error Message = " + Err.Description + vbCrLf + "Error Source = "_ 
+ Err.Source, MsgBoxStyle.Critical, "Error Occured")

    End Sub

Cags · Aug 2, 2006

I'm certainly not the best programmer on these forums, I'm also not a vb.Net programmer, but as normal I'm not going to let that get in the way of me voicing my opinions.

The first thing that case to mind upon looking at your code was to use a try statement instead of On Error GoTo. This wouldn't really alter your code in any logical sense but in my opinion is a more .Net way of doing it.

Visual Basic:

Try
    ' do stuff
Catch ex As Exception
    ' error message here
End Try

Personally whenever trying to decide if is a string is empty I'd compare it to String.Empty rather than checking if it equals(""). I don't know alot about the FileSystemObject, but as far as I can tell all your doing with it is checking the file exists. I would personally achieve this with the static (shared) function System.IO.File.Exists().

As far as reading in of the file goes that all looks vaguely familar from back when I was programming VB. I would imagine there is a more .Net way of doing this, but whether this would be using a FileStream or what I'm not sure.

In terms of efficiency, I have no real idea if my methods are any more efficient, a more .Net way of doing it or simply stupid ideas that I should be shot for (read: a personal preference).

Cags · Aug 2, 2006

Oh, I forgot to mention earlier, this site has a sister forum that deals exclusively with VB and VB.Net. I couldn't say how good it is as I've not used it for a few years.

http://www.xtremevbtalk.com

PlausiblyDamp · Aug 2, 2006

As a minimum you are probably better switching to the classes under System.IO for your file handling. A rough translation of your code would be something like

Visual Basic:

Public Sub LoadSettingsFromFile(ByVal FileName As String)
        On Error GoTo ErrorHandler

        If File.Exists(FileName) Then
            Dim tmpString As String
            Dim QuitReadingFile As Boolean = False
            Dim fs As FileStream = File.Open(FileName, FileMode.Open, FileAccess.Read)
            Dim br As New BinaryReader(fs)

            While QuitReadingFile = False
                tmpString = br.ReadString()
                If tmpString.IndexOf("DONE") > 0 Then
                    QuitReadingFile = True
                ElseIf tmpString.IndexOf("Variable1=") > 0 Then
                    var1 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                ElseIf tmpString.IndexOf("Variable2=") > 0 Then
                    var2 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                End If
            End While
            fs.Close()
        Else
            MsgBox("Can not find the file containing the variable." + vbCrLf + _
            "The Specified File = " + FileLocation_GameWindowTitle1)
        End If


        Exit Sub
ErrorHandler:
        MsgBox("An Error Occured in LoadSettingsFile SubRoutine." _
+ vbCrLf + "Error Number = " + Err.Number.ToString + vbCrLf + _
 "Error Message = " + Err.Description + vbCrLf + "Error Source = " _
+ Err.Source, MsgBoxStyle.Critical, "Error Occured")

    End Sub

although you might also want to use messagebox.Show() rather than msgbox and a Try ... Catch rather than On Error Goto.

As this is using native classes it should remove the need for interop with the FSO library.

If the file has one entry per line then you could make this easer by using a streamreader

Visual Basic:

        On Error GoTo ErrorHandler

        If File.Exists(FileName) Then
            Dim tmpString As String
            Dim QuitReadingFile As Boolean = False
            Dim sr As New StreamReader(FileName)

            While QuitReadingFile = False
                tmpString = sr.ReadLine()
                If tmpString.IndexOf("DONE") > 0 Then
                    QuitReadingFile = True
                ElseIf tmpString.IndexOf("Variable1=") > 0 Then
                    var1 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                ElseIf tmpString.IndexOf("Variable2=") > 0 Then
                    var2 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                End If
            End While
            sr.Close()
        Else
            MsgBox("Can not find the file containing the variable." + vbCrLf + _
            "The Specified File = " + FileLocation_GameWindowTitle1)
        End If

        Exit Sub
ErrorHandler:
        MsgBox("An Error Occured in LoadSettingsFile SubRoutine." _
+ vbCrLf + "Error Number = " + Err.Number.ToString + vbCrLf + _
 "Error Message = " + Err.Description + vbCrLf + "Error Source = " _
+ Err.Source, MsgBoxStyle.Critical, "Error Occured")

    End Sub

or even

Visual Basic:

   Public Sub LoadSettingsFromFile(ByVal FileName As String)
        On Error GoTo ErrorHandler

        If File.Exists(FileName) Then
            Dim tmpString As String
            Dim sr As New StreamReader(FileName)


            tmpString = sr.ReadToEnd()

            Dim s() As String = tmpString.Split(Environment.NewLine)

            For Each line As String In s
                If line.StartsWith("Variable1=") Then
                    var1 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                ElseIf line.StartsWith("Variable2=") Then
                    var2 = tmpString.Remove(0, tmpString.IndexOf("=") + 1)
                End If
            Next
            sr.Close()
        Else
            MsgBox("Can not find the file containing the variable." + vbCrLf + _
            "The Specified File = " + FileLocation_GameWindowTitle1)
        End If

        Exit Sub
ErrorHandler:
        MsgBox("An Error Occured in LoadSettingsFile SubRoutine." _
+ vbCrLf + "Error Number = " + Err.Number.ToString + vbCrLf + _
 "Error Message = " + Err.Description + vbCrLf + "Error Source = " _
+ Err.Source, MsgBoxStyle.Critical, "Error Occured")

    End Sub

would probably work.

Even easier though would be to use the built in XML serialisation support, or if you are using 2005 then this is done for you via the Settings tab of the project's property page.

NeuralJack · Aug 2, 2006

Thanks guys , this is exactly what i need.
I've never really used Try/Catch before. I'll look that up and probably go with it.
I'll go with the StreamReader too. It makes things look smoother.

I also noticed I dont need FSO to see if a file exists, that's nice.

How do you guys post in that nice VB.net code font?

Cags · Aug 2, 2006

The following page lists most of the code tags available, though funnily enough I don't see VB listed. Anyway, the tag required is the letters vb, just checkout the following link to see how tags work.

http://www.xtremedotnettalk.com/misc.php?do=bbcode

NeuralJack · Aug 2, 2006

Thanks here's what i got now. I couldnt use the For each Line.. statement because it said Line is no longer supported or something. so i used "i" to cycle through the array.
I like the split command, i had never used it before

I do a lot of programming, but not as a professional, so I dont pick up on a lot of things other may learn on the job. Most of that legacy VB stuff i was using because that's how it was in many many examples in the "Search" documentation that is provided with VB.Net. They have TONS of legacy stuff in there and they dont say it's old-school

Visual Basic:

    Public Sub LoadFile(ByVal FileName As String)
        Try
            If File.Exists(FileName) Then
                Dim tmpString As String
                Dim sr As New StreamReader(FileName)
                tmpString = sr.ReadToEnd
                Dim s() As String = tmpString.Split(Environment.NewLine)

                Dim i As Integer
                For i = 0 To s.GetUpperBound(0)
                    If s(i).StartsWith("var1=") Then
                        var1 = s(i).Remove(0, s(i).IndexOf("=") + 1)
                    ElseIf s(i).StartsWith("var2=") Then
                        var2 = s(i).Remove(0, s(i).IndexOf("=") + 1)
                    End If
                Next
                sr.Close()
            Else
                MessageBox.Show("Can not find the file." _
                    + vbCrLf + "The Specified File = " + FileName, _
                    "Can Not Find Specified File", MessageBoxButtons.OK, MessageBoxIcon.Error)
            End If
        Catch ex As Exception
            MessageBox.Show("An Error Occured." + vbCrLf _
                + vbCrLf + "Error Message: " + ex.Message _
                + vbCrLf + "Error Source: " + ex.Source _
                + vbCrLf + "Method that threw exception: " + ex.TargetSite.GetCurrentMethod.Name _
                , "An Error Occured", MessageBoxButtons.OK, MessageBoxIcon.Error)
        End Try

    End Sub

Cags · Aug 2, 2006

Just so you know you can still use a For Each loop, though it will be no more efficient than the method you are using. Some people find it easier to read as code though. For your code it would be something like the following...

Visual Basic:

For Each curString As String In s
    If curString.StartsWith("var1=") Then
        var1 = curString.Remove(0, curString.IndexOf("=") + 1)
    ElseIf curString.StartsWith("var2=") Then
        var2 = curString.Remove(0, curString.IndexOf("=") + 1)
    End If
Next

NeuralJack · Aug 2, 2006

I'd like to do that but it's giving me a syntax error with "As" underlined. I've tried a few things but no-go.

Cags said:
Just so you know you can still use a For Each loop, though it will be no more efficient than the method you are using. Some people find it easier to read as code though. For your code it would be something like the following...

Visual Basic:

For Each curString As String In s If curString.StartsWith("var1=") Then var1 = curString.Remove(0, curString.IndexOf("=") + 1) ElseIf curString.StartsWith("var2=") Then var2 = curString.Remove(0, curString.IndexOf("=") + 1) End If Next

PlausiblyDamp · Aug 2, 2006

Try

Visual Basic:

dim curString as string
Each curString In s
    If curString.StartsWith("var1=") Then
        var1 = curString.Remove(0, curString.IndexOf("=") + 1)
    ElseIf curString.StartsWith("var2=") Then
        var2 = curString.Remove(0, curString.IndexOf("=") + 1)
    End If
Next

the syntax posted needs .Net 1.1 or higher.

NeuralJack · Aug 2, 2006

Thanks - the Each, alone, didnt work but this did ( For Each curString In s). I have .Net 2.0 installed as can be seen in Add/Remove progs.. but i'm using VB.Net 2003.
Any mention of "As" or "String" didnt do it for me. I like this method more because it looks smoother

Visual Basic:

              Dim curString As String
                For Each curString In s
                    If curString.StartsWith("var1=") Then
                        var1 = curString.Remove(0, curString.IndexOf("=") + 1)
                    End If
                Next

NeuralJack · Aug 2, 2006

I got rid of the tmpString variable too by doing this:

Visual Basic:

Dim s() As String = sr.ReadToEnd.Split(Environment.NewLine)

ideally the function would take less memory now.. not that most settings files have a noticable amount of data to worry about.

Thanks for the help guys

NeuralJack · Aug 2, 2006

I'll probably head on over to http://www.xtremevbtalk.com/ to do most my posting from now on because it's probably more on subject there. Unless someone would think that i'd get better info from these forums?

I'm extremely surprised to see so many people as currently viewing Legacy VB threads though. I mean .net came out about ,what, 6 yrs ago? Legacy VB programmer numbers seems to still dwarf .net programmers.

I'll be known as NeuralJack over there. Thanks again.

Cags · Aug 2, 2006

Having .Net 2.0 on the add/remove panel, simply tells you that you have the version 2.0 of the the .Net Framework on your PC, allowing you to run applications developed in .Net 2.0 by other developers. If you are using VS 2003 you are programming using either .Net 1.1 or .Net 1.0. From what PlausiblyDamp was saying the line "For Each curString As String In s" not working implies you are using .Net 1.0, you might want to consider getting the .Net 1.1 update.

NeuralJack · Aug 2, 2006

ok thanks, i'll do that

Cags said:
Having .Net 2.0 on the add/remove panel, simply tells you that you have the version 2.0 of the the .Net Framework on your PC, allowing you to run applications developed in .Net 2.0 by other developers. If you are using VS 2003 you are programming using either .Net 1.1 or .Net 1.0. From what PlausiblyDamp was saying the line "For Each curString As String In s" not working implies you are using .Net 1.0, you might want to consider getting the .Net 1.1 update.

PlausiblyDamp · Aug 2, 2006

There is no reason at all to feel you need to move to http://www.xtremevbtalk.com/ for future questions - we have nothing against VB.Net here (in fact several of the regular posters are VB developers primarily). Often the issues are more to do with the .Net framework rather than the language syntax.

You are probably going to find answers over here will have less 'legacy' VB functionaility and more '.Net' functionality though.

NeuralJack · Aug 2, 2006

This is Jbob, I wanted to keep the same Login Name for both of these forums. I do intend to use as little legacy code as possible so i'll be hanging out a lot here too. Thanks for the info.

Efficient Programming Help - FileOpen

NeuralJack

Centurion

Cags

Contributor

Cags

Contributor

PlausiblyDamp

Administrator

NeuralJack

Centurion

Cags

Contributor

NeuralJack

Centurion

Cags

Contributor

NeuralJack

Centurion

PlausiblyDamp

Administrator

NeuralJack

Centurion

NeuralJack

Centurion

NeuralJack

Centurion

Cags

Contributor

NeuralJack

Centurion

PlausiblyDamp

Administrator

NeuralJack

Centurion