2
\$\begingroup\$

Here is some code that I put together for an app I am working on that loads 2 XML files and a large CVS file to a dataset at startup. It runs fairly fast, but I would like a second opinion on what I could do to either make it more consise or faster. I'm newer to .NET as well, so if you see anything that isn't very ."net ish" let me know!

#Region "Imports"
 Imports System.IO
 Imports System.Xml
 Imports System.Text
 Imports System.Threading
#End Region
 Class clsLoadTables
#Region "Properties and Shared Variables"
 Private Shared pathTableTwo As String = My.Settings.pathTableTwo
 Private Shared pathMainTable As String = My.Settings.pathMainTable
 Private Shared pathBeneLifeExp As String = My.Settings.pathBeneLifeExp
 Private _ds As New DataSet
 Public Property ds() As DataSet
 Get
 Return _ds
 End Get
 Set(ByVal value As DataSet)
 _ds = value
 End Set
 End Property
#End Region
#Region "Constructors"
 Sub New()
 loadBeneLifeExpTable()
 loadMainRMDTable()
 loadCSVTableII()
 End Sub
#End Region
#Region "ClassMethods"
 Public Sub loadCSVTableII()
 Dim dt As DataTable = ds.Tables.Add("TableII")
 Dim line As String = String.Empty
 Dim counter As Short = 0
 Dim reader As New StreamReader(pathTableTwo)
 Dim errorString As New StringBuilder
 Try
 errorString.Append("The tableII csv file did not load properly")
 errorString.Append(Environment.NewLine & Environment.NewLine)
 errorString.Append("Make syre the tabel_II.csv file is in the project folder")
 Catch ex As Exception
 Throw
 End Try
 Try
 While Not reader.EndOfStream()
 line = reader.ReadLine()
 Dim lineSep As List(Of String) = line.Split(",").ToList
 If Not counter = 0 Then
 dt.Rows.Add(lineSep.ToArray)
 counter += 1
 Else
 For Each value As String In lineSep
 dt.Columns.Add(value)
 Next
 counter += 1
 End If
 End While
 Dim primarykey(0) As DataColumn
 primarykey(0) = dt.Columns("Ages")
 dt.PrimaryKey = primarykey
 Catch ex As FileNotFoundException
 MessageBox.Show(errorString.ToString)
 Throw
 Catch ex As Exception
 Throw
 Finally
 reader.Close()
 End Try
 End Sub
 Public Sub loadMainRMDTable()
 Dim tempDs As New DataSet 
 Dim dt As New DataTable
 Dim settings As New XmlReaderSettings
 Dim errorString As New StringBuilder
 Try
 errorString.Append("The RMD table did not load properly!")
 errorString.Append(Environment.NewLine & Environment.NewLine)
 errorString.Append("Make sure that the file 'MainRMDTable.xml' is in the project folder")
 Catch ex As Exception
 Throw
 End Try
 Try
 Dim xmlFile As XmlReader
 xmlFile = XmlReader.Create(pathMainTable, settings)
 tempDs.ReadXml(xmlFile)
 dt = tempDs.Tables("Age")
 dt.TableName = "MainRMDTable"
 xmlFile.Close()
 Dim primarykey(0) As DataColumn
 primarykey(0) = dt.Columns("age")
 dt.PrimaryKey = primarykey
 ds.Merge(tempDs)
 Catch ex As FileNotFoundException
 Throw
 Catch ex As Exception
 MessageBox.Show(errorString.ToString)
 Throw
 Finally
 errorString.Clear()
 tempDs.Clear()
 End Try
 End Sub
 Public Sub loadBeneLifeExpTable()
 Dim dt As New DataTable
 Dim settings As New XmlReaderSettings
 Dim errorString As New StringBuilder
 Try
 errorString.Append("The bene life expectancy table did not load properly ")
 errorString.Append(Environment.NewLine & Environment.NewLine)
 errorString.Append("Make sure that the file 'beneLifeExpectancyTable.xml' is in the project folder")
 Catch ex As Exception
 Throw
 End Try
 Try
 Dim xmlFile As XmlReader
 xmlFile = XmlReader.Create(pathBeneLifeExp, settings)
 ds.ReadXml(xmlFile)
 dt = ds.Tables("Age")
 dt.TableName = "BeneLifeExpectancyTable"
 xmlFile.Close()
 Dim primarykey(0) As DataColumn
 primarykey(0) = dt.Columns("BeneLifeExpectancyTable")
 dt.PrimaryKey = primarykey
 Catch ex As Exception
 MessageBox.Show(errorString.ToString)
 MessageBox.Show(ex.Message & ex.StackTrace())
 Throw
 Finally
 errorString.Clear()
 End Try
 End Sub
#End Region
 End Class
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Mar 18, 2012 at 23:17
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I'm not a vb.net expert but do you need to do a line.ToList() on the string when you are doing a ToArray() later on. Seems like possible duplication to me? \$\endgroup\$ Commented Mar 21, 2012 at 19:16
  • \$\begingroup\$ Dreza, yeah you are right. The reason I did it this way is that I am trying to not use any of the vb array methods if I can get away with it. I want to get in the habit of using the modern .Net components. But in this case you are correct, I should just declare lineSep as an array becuase that is what line.split will return. Also, since you can't add a generic list directly to a datatable I could eliminate the dt.row.Add(lineSep.ToArray) statement as well. Good catch. \$\endgroup\$ Commented Mar 22, 2012 at 0:23

1 Answer 1

2
+50
\$\begingroup\$

Move code that displays Message Boxes up the call hierarchy. Make better use of exceptions. Use String Builders only when you append a lot, not just a couple of lines, try using String.Format. Follow official VB.NET naming conventions "Begin each separate word in a name with a capital letter".

Make one generic method instead of loadMainRMDTable and loadBeneLifeExpTable by passing file path, table name and column name(s) as parameters.

Some code to illustrate:

Sub New()
 Try
 LoadBeneLifeExpTable()
 LoadMainRMDTable()
 LoadCSVTableII()
 Catch ex As Exception
 MessageBox.Show(ex.ToString())
 End Try
End Sub
Public Sub LoadBeneLifeExpTable()
 Dim dt As New DataTable
 Dim settings As New XmlReaderSettings
 Try
 Dim xmlFile As XmlReader
 xmlFile = XmlReader.Create(pathBeneLifeExp, settings)
 ds.ReadXml(xmlFile)
 dt = ds.Tables("Age")
 dt.TableName = "BeneLifeExpectancyTable"
 xmlFile.Close()
 Dim primarykey(0) As DataColumn
 primarykey(0) = dt.Columns("BeneLifeExpectancyTable")
 dt.PrimaryKey = primarykey
 Catch ex As Exception
 //' Using String.Format would be even better
 Throw New Exception("The bene life expectancy table did not load properly " + _
 Environment.NewLine + Environment.NewLine + _
 "Make sure that the file '" + "' is in the project folder",
 ex)
 End Try
End Sub

PS: try not to use VB(.NET) - there are better languages; get ReSharper (it will give you some hints on how to make code better).

answered Mar 21, 2012 at 10:15
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the analysis. When I use just the 'throw' statement in my methods it is because the class that is calling the methods is already using a try statement. I thought that I had to do it this way to retain the ex.stacktrace all the way up the code. I've been looking around for some good vb.net documentation about nested exceptions but cannot find any. Do you know of any good links? BTW - I am using VB.Net because I have to for a few projects at work. I fully intend on getting up to speed with C# once I can write basic apps in vbnet. \$\endgroup\$ Commented Mar 22, 2012 at 0:18
  • \$\begingroup\$ "Nested exceptions" is probably not a widely recognized term and there is not much to it. You are correct that "Throw" allows you to preserve the stack trace as opposed to "Throw ex". However you can also set ex.InnerException property as I did via constructor to specify the underlying exception that caused current exception. This way you can build a "hierarchy" of exceptions - e.g. from more concrete to more generic. When using ex.ToString() it will produce a string containing messages and stack traces of current exception along with all its inner exceptions. It's OOP - not a language feature \$\endgroup\$ Commented Mar 22, 2012 at 9:49

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.