Efficient Programming Help - FileOpen

[NeuralJack]

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:

 

   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]

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.

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]

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]

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

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

       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

  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]

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]

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]

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

 

   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]

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...

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]

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.

 

 

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...

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]

Try

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]

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

 

             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]

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

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]

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]

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]

ok thanks, i'll do that

 

Having .Net 2.0 on the add/remove panel' date=' 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.[/quote']

[PlausiblyDamp]

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.

Edited August 2, 2006 by PlausiblyDamp

[NeuralJack]

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.