Strongly typed datasets - am I doing this right?

ikantspelwurdz

Well-known member
Joined
Dec 8, 2009
Messages
49
Programming Experience
1-3
Here's the code for a program that rolls a 20-sided die 1,001 times, and then saves the list of rolls to an Access database using a very simple strongly typed dataset table adapter.

VB.NET:
Module Module1

    Sub Main()
        Dim rolls As New List(Of Integer)

        Dim r As New Random
        For i As Integer = 0 To 1000
            rolls.Add(r.Next(20) + 1)
        Next

        Dim DiceAdapter As New DiceDataSetTableAdapters.rollsTableAdapter
        Dim rollsInserted As Integer = 0
        For Each roll As Integer In rolls
            DiceAdapter.Insert(roll)
            rollsInserted += 1
            If rollsInserted Mod 10 = 0 Then
                Dim p As Integer = 100 * rollsInserted / rolls.Count
                Console.Clear()
                Console.WriteLine(p & "%")
            End If
        Next

    End Sub

End Module

This works, but it works quite slowly, taking about three minutes to complete. Am I even doing this right? Is there a way to make this faster? My solution is attached.
 

Attachments

  • DiceDB.zip
    36.6 KB · Views: 13
First up, rather than this:
rolls.Add(r.Next(20) + 1)
you would normally do this:
rolls.Add(r.Next(1, 21))
Also, while there's nothing necessarily wrong with your loop, you could also do this:
Dim rolls = Enumerable.Range(0, 1001).Select(Function(n) r.Next(1, 21)).ToArray()
As for inserting into the database, you really shouldn't be using a loop to call Insert like that. Firstly, that code is going to open and close a database connection 1001 times. If you were going to use that loop then you'd open the connection explicitly before you started and close it when you're done.

Also, if you were going to use a loop then what's the List for? Why use one loop to populate the List and another read it? Why not just do away with the List and use one loop to generate and save the data?

I wouldn't use a loop at all though. I would populate the appropriate DataTable with all the rolls and then save it all in one batch with a call to Update. Use DBDirect methods like Insert when you have just one record to save. If you have lots, always use a DataTable.
 
that code is going to open and close a database connection 1001 times. [...] I would populate the appropriate DataTable with all the rolls and then save it all in one batch with a call to Update.
That is what I wanted to know. Thanks.

The multiple loops is my idea of modular design. I'd rather have two blocks of code that each do one thing than one block of code that does two things if the performance requirement is roughly the same. The actual project where I have this problem is much more complicated, and it made sense to me to break it down into this:
Step 1: Generate lists of data
Step 2: Write them to tables

Now it looks like this:
Step 1: Generate lists of data
Step 2: Put them into a strongly typed data table
Step 3: Write it to the table

And my code looks like this and runs much faster:
VB.NET:
    Sub Main()
        Dim rolls As New List(Of Integer)

        Dim r As New Random
        For i As Integer = 0 To 1000
            rolls.Add(r.Next(1, 21))
        Next

        Dim DiceTable As New diceDataSet.rollsDataTable
        For Each roll As Integer In rolls
            DiceTable.AddrollsRow(roll)
        Next

        Dim DiceAdapter As New diceDataSetTableAdapters.rollsTableAdapter
        DiceAdapter.Update(DiceTable)

    End Sub
 
You've still got two loops when you only need one. What's the point of the List if you're just going to put the values in in one loop and then take them out in the other? Why can't you just use one loop and put the values straight into the DataTable?
 
It's certainly doable with a single loop, but I feel doing it with two loops is preferable for the reasons I outlined above.

Also, there's re-usability. A function such as this can be used independently of your database schema, or even without any database at all:
VB.NET:
    Public Function GetListOfRolls(ByVal dieSides, ByVal maxRolls) As List(Of Integer)
        Dim rolls As New List(Of Integer)

        Dim r As New Random
        For i As Integer = 1 To maxRolls
            rolls.Add(r.Next(1, dieSides + 1))
        Next

        Return rolls

    End Function

A function such as this:
VB.NET:
    Public Function GetDataTableOfRolls(ByVal dieSides, ByVal maxRolls) As diceDataSet.rollsDataTable
        Dim DiceTable As New diceDataSet.rollsDataTable

        Dim r As New Random
        For i As Integer = 1 To maxRolls
            DiceTable.AddrollsRow((r.Next(1, dieSides + 1)))
        Next

        Return DiceTable

    End Function
is much more limited in where you can use it.
 
OK, I looked at the code but not the rest of your post, which was a mistake. If those two processes are in different methods then it obviously makes perfect sense to have two separate loops.
 
Back
Top