0

I am making a website and I have a for each loop that is taking 2.5 minutes to run through.

I am wondering if anyone knows how i can speed it up. Both of the if statements inside of the loop have to remain there.

 Protected Sub WebDataGrid1_InitializeRow(sender As Object, e As Infragistics.Web.UI.GridControls.RowEventArgs) Handles WebDataGrid1.InitializeRow
        Dim SqlString As String = ""
        Dim TerrMapTable As New DataTable
        Dim Terr As String = ""
        Dim RgnMgr As String = ""
        Dim Reg As String = ""
        Dim TerrMap As String = ""

        For Each GridRecord As GridRecord In WebDataGrid1.Rows
            Terr = GridRecord.Items.FindItemByKey("ABAN8").Value
            RgnMgr = GridRecord.Items.FindItemByKey("RgnMgr").Value
            If Terr < 9000 Then
                Terr = "T" & Terr
            Else
                Terr = "TA1" & Right(Terr, 2)
            End If

            SqlString = "Select distinct Terr, Region from TerrReg WHERE Terr='" & Terr & "'"

            TerrMapTable = OleFun.GetMyDataTableString(SqlString, 4)

            If TerrMapTable.Rows.Count <> 0 Then
                Reg = TerrMapTable(0)(1)

                TerrMap = "<a href=""/Mapper/Territory_Maps/" & Reg & "/" & Terr & "/" & Terr & "-" & Reg & "-" & "Territory.htm"" target=""_Blank""><b>PIC</b></a>"

                GridRecord.Items.FindItemByKey("TerrMap").Text = TerrMap
            Else
            End If
        Next

    End Sub
Mederic
  • 1,949
  • 4
  • 19
  • 36
  • 2
    Why are you going through every row in the first place? – Mederic Jun 29 '17 at 13:04
  • 2
    This question is suited for https://codereview.stackexchange.com... – Trevor Jun 29 '17 at 13:15
  • Look at this [SO Q&A](https://stackoverflow.com/questions/33302962/performance-difference-between-looping-range-vs-looping-array). – MikeJRamsey56 Jun 30 '17 at 00:18
  • 1
    You should not run a query in every iteration and I suspect this is your only problem. Try to get all your data first then grab what you need from memory. How large is your table and how long does one query take? Better yet, show the execution plan. – Michael Z. Jun 30 '17 at 01:38

2 Answers2

2

In the event handler for InitializeRow, which I would expect to be called for a single row, you are looping through all of the rows. As a result, I suspect that you have turned an operation that should be linear into the number of rows (with the event firing once for each row, and you operate only on that row) into an operation that is quadratic in the number of rows (for each row, you loop through each row).

Based on the event name, I don't think you should have the For Each loop at all. It looks like you should be getting the appropriate row from the event arguments and operating on that row only.

Craig
  • 2,248
  • 1
  • 19
  • 23
1

This could be better suited for code review but here are my thought.

  • Use string building to concatenate strings
  • Don't concatenate sql! Use parameters.
  • Get all the unique Terr and do one query.
  • Database queries must be your bottleneck.
  • Terr seems like a mix of number and string. Try option strict on.
the_lotus
  • 12,668
  • 3
  • 36
  • 53