1

I'm executing a query which returns me around 400 records (orders) and I display them in DataGridView. Then I have to check each row and make a green every row which exists in other MySQL database (by ID). I think I'm not doing this optimally. Here is what I am doing:

For Each oRow As DataGridViewRow In dgv.Rows
       Dim orderNumber As Integer = oRow.Cells(0).Value

       Dim exist As Boolean = mySql.CheckIfExists(shop, orderNumber)

       If Not exist Then
             Continue For
       End If

       If exist Then
             oRow.DefaultCellStyle.BackColor = Color.LightGreen
             oRow.DefaultCellStyle.SelectionBackColor = Color.Green
             oRow.DefaultCellStyle.SelectionForeColor = Color.LightCoral
             Continue For
       End If
Next

Here is CheckIfExists() method in MySQL class:

Public Function CheckIfExists(ByVal shop As String, ByVal orderNumber As Integer) As Boolean

        Dim dt As New DataTable
        Dim sql As String = "" 'sql query'

        Connect(mSOH) 'msoh is a connection string from My.Settings'

        Dim sqlCommand As New MySqlCommand
        With sqlCommand
            .Connection = connection
            .CommandText = sql
        End With


        Try
            Dim sqlReader As MySqlDataReader = sqlCommand.ExecuteReader()
            While sqlReader.Read()
                Return True
            End While

        Catch ex As MySqlException
            Logi.LogInfo(ex)
        Catch ex As Exception
            Logi.LogInfo(ex)
        Finally
            Disconnect()
        End Try

        Return False
End Function

And Connect and Disconnect methods if they are important:

    Private Sub Connect(shop As String)

            Select Case shop
                Case "jablotron"
                    connection = New MySqlConnection(csJablotron)
                Case "bcs"
                    connection = New MySqlConnection(csBCS)
                Case "mSOH"
                    connection = New MySqlConnection(csmSOH)
                Case Else
                    connection = New MySqlConnection(shop)
            End Select

            Try
                connection.Open()
            Catch ex As MySqlException
                Logi.LogInfo(ex)
            Catch ex As Exception
                Logi.LogInfo(ex)
            End Try
        End Sub


        Private Sub Disconnect()
            Try
                connection.Dispose()
            Catch ex As MySqlException
                Logi.LogInfo(ex)
            Catch ex As Exception
                Logi.LogInfo(ex)
            End Try
    End Sub

So if I try to check each row by this way it takes some time (around <1 second if database is on localhost, and around 30 second if database is on remote server and I try to connect via VPN). Is this way being optimal - checking each row like this? Is it a good approach? Please give me some tip and advice :) I know that code is in VB.NET, but guys from C# can also help me :)

Branislav Lazic
  • 14,388
  • 8
  • 60
  • 85
XardasLord
  • 1,764
  • 2
  • 20
  • 45
  • 1
    Use this logic: Pre-load your order numbers into lists and cache them. For how long? - this is something for you to decide, based on how volatile the data is. Then check against those lists. As well, It will be faster if you just parse your grid data by database and build `In` statements for each of your databases and load all needed records at once. Then only loop through already loaded data. And only relative data will be loaded. – T.S. Nov 26 '14 at 00:22

2 Answers2

1

Use this logic: Pre-load your order numbers into lists and cache them. For how long? - this is something for you to decide, based on how volatile the data is. Then check against those lists.

As well, It will be faster if you just parse your grid data by database and build In statements for each of your databases and load all needed records at once. Then only loop through already loaded data. And only relative data will be loaded.

Dim existingOrders as New List(of String)
Dim shopOrders as New List(of String)

/* Collect orders into list  */
For Each oRow As DataGridViewRow In dgv.Rows

   Dim orderNumber As Integer = oRow.Cells(0).Value
   shopOrders.Add(orderNumber)

Next
/* Get and accumulate existing orders from each shop */
For Each shop as string In shops
    GetOrdersByShop(shop, shopOrders, existingOrders)
Next


For Each oRow As DataGridViewRow In dgv.Rows
    If existingOrders.Contains(oRow.Cells(0).Value) Then
         oRow.DefaultCellStyle.BackColor = Color.LightGreen
         oRow.DefaultCellStyle.SelectionBackColor = Color.Green
         oRow.DefaultCellStyle.SelectionForeColor = Color.LightCoral
    End If
Next


/* ---------------------------------------------------- */


Private Sub GetOrdersByShop(shop As string, shopOrders As List(of String), 
    existingOrders as New List(of String)) 
    /* here, turn shopOrders into In('ord1', 'ord2', . . . )

    and after running query add found orders to  existingOrders */


End Sub

You can also use dictionary - it will make search work faster than list and you may store which order is in which shop, etc.

T.S.
  • 18,195
  • 11
  • 58
  • 78
  • Oh great answer! It helps me a lot. Firstly this code is much faster, because there is only one connection to the database as you said :) Secondly, very clean code, nice and better approach I guess. This is what I've needed! Btw, `KeyValuePair` is ok? Or better use `dictionary`? – XardasLord Nov 26 '14 at 01:35
  • 1
    `KeyValuePair` is basically a single unit in `Dictionary`. So, when you iterate dictionary in `ForEach` you get key-value pair. Enjoy! – T.S. Nov 26 '14 at 01:57
1

First off, a DataGridView is a VIEW object. You should use it as such. For data, you use a data object.

Personally I would do something like this:

Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
    Dim dtMain = SqlReadDatatable(firstConnString, "SELECT * FROM SomeTable")

    Dim lstGreenIds = (From row In SqlReadDatatable(secondConnString, "SELECT ID FROM SomeOtherTable")
                       Select CInt(row("ID"))).ToList

    dtMain.Columns.Add(New DataColumn("Exists", GetType(Boolean)))

    For Each dtRow As DataRow In (From row In dtMain.Rows Where lstGreenIds.Contains(CInt(row("ID"))))
        dtRow("Exists") = True
    Next

    DataGridView1.DataSource = dtMain
End Sub

Private Function SqlReadDatatable(ByVal connStr As String, ByVal query As String) As DataTable
    Dim dt As New DataTable
    Using conn As New SqlConnection(connStr) : conn.Open()
        Using cmd As New SqlCommand(query, conn)
            dt.Load(cmd.ExecuteReader)
        End Using
    End Using
    Return dt
End Function

Private Sub DataGridView1_RowPostPaint(sender As Object, e As DataGridViewRowPostPaintEventArgs) Handles DataGridView1.RowPostPaint
    Dim dgv = CType(sender, DataGridView)
    Dim dgvRow = dgv.Rows(e.RowIndex)
    Dim dtRow = CType(dgvRow.DataBoundItem, DataRow)

    If dtRow("Exists") Then
        dgvRow.DefaultCellStyle.BackColor = Color.LightGreen
        dgvRow.DefaultCellStyle.SelectionBackColor = Color.Green
        dgvRow.DefaultCellStyle.SelectionForeColor = Color.LightCoral
    End If
End Sub

This cleanly separates data from visualisation. The color is only applied when the row comes into view, no need to iterate over the whole bunch at once.

EDIT: I should also note that the cleanest way to do this would be server-side. In MySQL you can use cross-database queries if both databases are on the same server. If they are on different servers, you can create a FEDERATED TABLE on one of the servers and connect it to the remote server. You can read about those here: http://dev.mysql.com/doc/refman/5.0/en/federated-storage-engine.html . In the case of two databases on the same server, you could just do a join:

SELECT f.*, NOT ISNULL(s.OrderNum) AS Exists
FROM 'firstdb'.'firsttable' f
LEFT JOIN 'seconddb'.'secondtable' s ON f.OrderNum = s.OrderNum

And then use the Exists column in code to color the row.

Drunken Code Monkey
  • 1,796
  • 1
  • 14
  • 18