Bus Driver Analysis Application: speeding up queries, for loops, and proper syntax

Question

I'm working on a project, at work, called the "Bus Driver." The bus driver is a material handler that goes to 9 different assembly lines, on the production floor, and picks up finished goods (refurbished receivers) and drops them off at a converyer belt where they are then shrinked wrapped and sent to be palletized.

The application I'm working on tracks each time he makes one complete route and then stores his stats in an SQL DB.

The application that records his route stats works fine but I have another application called, "The Bus Driver Analyzer." This application gets all the Bus driver's data and produces 4 charts.

1. Chart 1: Route Efficiency (52 weeks are shown, in this chart, on the X-value)
2. Chart 2: On time delivery Rates (52 weeks are shown, in this chart, on the X-value)
3. Chart 3: Histogram Chart (there is a radio button for a YTD or Weekly option)
4. Chart 4: Stack Ranking chart (top 10)

The two charts below are the two charts i want to focus on for this question.

My problem with this application is that I have 52 labels, on the windows form, to hold his route efficiencies for each week (seen on your left.) They are called "LblWkEff1" and goes up to "LblWkEff52" and then I have another 52 labels, on the windows form to hold his On time delivery rates (Seen on your right.) They are called "lblDeliveryStat1" and go up to "lblDeliveryStat52."

The code I have to retrieve these results is horrendeous, from what i've been told, on this site, and I don't disagree. I'm still new to programming so what I have written is by no means clean and perfect.

here is my code:

Dim RESULT1 As Decimal 'declare this as global
Dim RESULT2 As Decimal 'declare this as global

Private Sub Week(ByVal week As Integer)

    Dim queryString As String = "SELECT " & _
                                " (SELECT CAST(SUM(TARGET_SECONDS) AS DECIMAL)/ CAST(SUM(ROUTE_SECONDS) AS DECIMAL) FROM dbo.APE_BUSDRIVER_MAIN WITH(NOLOCK) WHERE WEEK_TIME = " & week & " AND APE_AREA_OBJID = " & lblAreaOBJID.Text & " AND EMPLOYEE_NAME = '" & cbEmployeeName.Text & "' AND YEAR_TIME = '" & cbYear.Text & "' AND ACTIVE = 1) AS RESULT1," & _
                                " (SELECT (SELECT CAST(COUNT(APE_BUSDRIVER_STATUS_OBJID) AS DECIMAL) FROM dbo.APE_BUSDRIVER_MAIN AS RESULT2 WHERE WEEK_TIME = " & week & " AND APE_AREA_OBJID = " & lblAreaOBJID.Text & " AND EMPLOYEE_NAME = '" & cbEmployeeName.Text & "' AND YEAR_TIME = '" & cbYear.Text & "' AND ACTIVE = 1 AND APE_BUSDRIVER_STATUS_OBJID = 1)/(SELECT CAST(COUNT(APE_BUSDRIVER_STATUS_OBJID) AS DECIMAL) FROM dbo.APE_BUSDRIVER_MAIN AS RESULT2 WHERE WEEK_TIME = " & week & " AND APE_AREA_OBJID = " & lblAreaOBJID.Text & " AND EMPLOYEE_NAME = '" & cbEmployeeName.Text & "' AND YEAR_TIME = '" & cbYear.Text & "' AND ACTIVE = 1)) AS RESULT2" & _
                                " FROM dbo.APE_BUSDRIVER_MAIN "

    Using connection As New SqlConnection(SQLConnectionStr)
        Dim command As New SqlCommand(queryString, connection)
        connection.Open()

        Dim reader As SqlDataReader = command.ExecuteReader()

        ' Call Read before accessing data. 
        If reader.HasRows Then
            While reader.Read()
                RESULT1 = reader("RESULT1")
                RESULT2 = reader("RESULT2")
            End While
        Else
            RESULT1 = 0
            RESULT2 = 0
        End If
        ' Call Close when done reading.
        reader.Close()
    End Using
End Sub    Private Sub LoadWeeklyStats()

    'LOOP AND  QUERY
    For i As Integer = 0 To 51
        Week(i + 1)
        Dim LabelWkEff As String = "LblWkEff" + (i + 1).ToString
        Dim myArray1 As Array = Controls.Find(LabelWkEff, False)
        Dim myControl1 As Label = myArray1(0)
        myControl1.Text = RESULT1
        'AND
        Dim LabelDeliveryStat As String = "lblDeliveryStat" + (i + 1).ToString
        Dim myArray2 As Array = Controls.Find(LabelDeliveryStat, False)
        Dim myControl2 As Label = myArray2(0)
        myControl2.Text = RESULT2
    Next
End Sub  

Private Sub LoadWeeklyStats() is what im using to hold the results and place them on the "LblWkEffXX" and "lblDeliveryStatXX" labels, on the windows form.

this process takes 5 seconds each time a new user is selected for reviewal and I know it has something to do with the SQL query and the for loop but I dont know how else to write the code to get the results I want more efficiently.

Any feedback on how to rewrite the code or what other option i can perform to achieve the same results in a much quicker time would be most appreciated.

please let me know if you need more information.

Answer 1

From a cursory look, I would suggest that there is nothing inherently wrong with what you are trying to do. I think the biggest problem is in the opening of 52 sql connections, which is a relatively expensive process. Therefore I suggest that the simplest efficiency savings would be to either amend your query to get all the data back at once, or to simply pass the Connection instance into the Week() method.

Answer 2

Charting software, such as dundas and I'm sure even the built-in charting types, usually take in a multi-dimensional array of values: one value being the X axis value and one being the Y value axis. You should not need to set each label individually.

However, if you want to check where your efficiencies are lost in the current system, I'd comment out:

    Dim myArray1 As Array = Controls.Find(LabelWkEff, False)
    Dim myControl1 As Label = myArray1(0)
    myControl1.Text = RESULT1

and

    Dim myArray2 As Array = Controls.Find(LabelDeliveryStat, False)
    Dim myControl2 As Label = myArray2(0)
    myControl2.Text = RESULT2

This will tell you whether or not that 5 seconds is coming from the DB queries or if it's coming from the amount of Controls.Find calls you're making. You could just profile your code as well, but this is a pretty easy test. If it gets dramatically faster, then it's your Controls.Find and not your queries. If, however, it's still slow, then time to profile your query to see if a restructuring or an index is needed.

If it gets faster, your best bet is to create an array of controls on first load then use that array in subsequent fills. No need to find your controls each time -- they're not going to change!

Something like this workflow:

• Create two new List(Of Label) member variables in your class -- one to store the LabelWkEff values and one to store the LabelDeliveryStat values.
• On Form load, loop: For i As Integer = 0 To 51, doing the same code as before to find the controls.
• Instead of setting their label values, add them to their new respective member variables
• In the code you pasted, where before you were finding the control, use the i index to access the correct control from the new member variables, e.g. Dim myControl1 as Label = LabelWkEffs(i)

That should make things significantly faster, assuming your bottleneck is the Controls.Find.

Answer 3

Joseph, as many have mentioned, 52 round trips to database is not a good idea. Here is what I tried to to:

1. Create a stored procedure that takes parameter and gets the 52 week statistics for a given driver at once.

   Create procedure GetDriverStatsBetweenWeeks(

     @area_objid int,
     @emp_name varchar(255),
     @year_time int,
     @start_Week int = 1,
     @end_Week int = 52
   

   ) as begin

    declare @driver_stats table(weektime int, result1 decimal(38, 10), result2  decimal(38, 10))
   
    declare @currentWeek int
    set @currentWeek = @start_Week 
   
    while (@currentWeek <= @end_Week)
    begin
   
   insert into @driver_stats(weektime, result1, result2)
   select
       week_time
       ,(cast(sum(TARGET_SECONDS) AS DECIMAL)
         /cast(sum(ROUTE_SECONDS) AS DECIMAL)) as Result1
       ,(cast(sum((case when APE_BUSDRIVER_STATUS_OBJID = 1 then 1 else 0 end)) as decimal)
         /cast(sum((case when APE_BUSDRIVER_STATUS_OBJID <> 1 then 1 else 0 end)))) as Result2
   FROM 
       dbo.APE_BUSDRIVER_MAIN a
   WHERE 
       WEEK_TIME = @currentWeek AND APE_AREA_OBJID = @area_objid 
       AND EMPLOYEE_NAME = @emp_name and YEAR_TIME =  @year_time AND ACTIVE = 1
   group by week_time
   
   set @currentWeek = @currentWeek + 1
   end 
   
   select weektime, result1, result2 from @driver_stats
   

   end go

2. Create a data layer (dummy one) that connects to the sql server executes the above proc gets the results into a model

   Public Class ClassDriverStatistics

      Property Week As Integer = 0
      Property Result1 As Decimal = 0.0 ' rename this property to something like  RouteEffeciency
      Property Result2 As Decimal = 0.0 ' rename this property to the appropriate name
   
      Public Sub New()
      End Sub
   

   End Class

   Public Class MyDataLayer

   Public Shared Function GetWeeklyDriverStats(StartWeek Integer, _ EndWeek Integer, YearTime Integer, _ AreaObjId Integer, EmployeeName String) _

   as List(Of ClassDriverStatistics)
   
   Dim oDriverWeeklyStatsList As New List(Of ClassDriverStatistics)
   
   Using connection As New SqlConnection(SQLConnectionStr)
   
       Dim command As SqlCommand = New SqlCommand("GetDriverStatsBetweenWeeks", connection)
       connection.CommandType = CommandType.StoredProcedure
       connection.Open()
   
       Dim reader As SqlDataReader = command.ExecuteReader()
       Do While objReader.Read()
           Dim rec As ClassDriverStatistics = New ClassDriverStatistics()
           rec.Week = Convert.ToInt32(theObjReader("weektime"))
           rec.Result1 = Convert.ToDecimal(theObjReader("result1"))
           rec.Result2 = Convert.ToDecimal(theObjReader("result2"))
           oDriverWeeklyStatsList.Add(rec)
       Loop
       objReader.Close()
   
   End Using
   
   Return oDriverWeeklyStatsList
   

   End Function

   End Class

3. Obviously, not much changes to the LoadWeeklyStats() except to replace the loop of queries with a loop through the list of data model objects

   Private Sub LoadWeeklyStats()

   Dim weeklyStats As List(Of ClassDriverStatistics) = _ MyDataLayer.GetWeeklyDriverStats(1, 52, cbYear.Text, _ lblAreaOBJID.Text, cbEmployeeName.Text)

   For Each weekStat As String In weeklyStats

   Dim LabelWkEff As String = "LblWkEff" + weekStat.Week.ToString
   Dim myArray1 As Array = Controls.Find(LabelWkEff, False)
   Dim myControl1 As Label = myArray1(0)
   myControl1.Text = weekStat.Result1
   
   Dim LabelDeliveryStat As String = "lblDeliveryStat" + weekStat.Week.ToString
   Dim myArray2 As Array = Controls.Find(LabelDeliveryStat, False)
   Dim myControl2 As Label = myArray2(0)
   myControl2.Text = weekStat.Result2
   

   Next

   End Sub

Please bear in mind

1. This code is not tested because I couldn't
2. Some how, I couldn't get this code in stack overflow to be properly formatted
3. I'm good at C# but not at VB.NET, this is some thing I tried to provide an answer because this question is already posted earlier as performance-speeding-up-application

Please see if this improves any performance. Also, apply the other aspects that others here at SO gave above.