• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 236
  • Last Modified:

Make the Code faster

Hi

VB .Net Windows Application.
 The following code refers places af specific bednumber (X = 8, 10, 12 ....54) with a specific personnumber (HCV). The code runs when the application starts and every 15 minutes.
The code is working as it should but is slow. Is it possible to make it faster?

    Private Sub Hent()
        SuspendLayout()
        Dim Connection As New SqlConnection(Conn)
        Connection.Open()
        Dim X As Long
        For X = 8 To 54 Step 2
            Dim Command As New SqlCommand("SELECT HCV FROM Sengeplads WHERE Plads = '" & X & "'", Connection)
            Dim reader As SqlDataReader = Command.ExecuteReader()
            If reader.Read() Then

                If X = 8 And TextBox1.Text <> reader.GetString(0) Then
                    Me.TextBox1.Text = reader.GetString(0)
                    Me.TextBox2.Focus()
                ElseIf X = 10 And TextBox2.Text <> reader.GetString(0) Then
                    Me.TextBox2.Text = reader.GetString(0)
                    Me.TextBox4.Focus()
                ElseIf X = 12 And TextBox4.Text <> reader.GetString(0) Then
                    Me.TextBox4.Text = reader.GetString(0)
                    Me.TextBox3.Focus()
                ElseIf X = 14 And TextBox3.Text <> reader.GetString(0) Then
                    Me.TextBox3.Text = reader.GetString(0)
                    Me.TextBox9.Focus()
                ElseIf X = 16 And TextBox9.Text <> reader.GetString(0) Then
                    Me.TextBox9.Text = reader.GetString(0)
                    Me.TextBox12.Focus()
                ElseIf X = 18 And TextBox12.Text <> reader.GetString(0) Then
                    Me.TextBox12.Text = reader.GetString(0)
                    Me.TextBox21.Focus()
                ElseIf X = 20 And TextBox21.Text <> reader.GetString(0) Then
                    Me.TextBox21.Text = reader.GetString(0)
                    Me.TextBox29.Focus()
                ElseIf X = 22 And TextBox29.Text <> reader.GetString(0) Then
                    Me.TextBox29.Text = reader.GetString(0)
                    Me.TextBox25.Focus()
                ElseIf X = 24 And TextBox25.Text <> reader.GetString(0) Then
                    Me.TextBox25.Text = reader.GetString(0)
                    Me.TextBox26.Focus()
                ElseIf X = 26 And TextBox26.Text <> reader.GetString(0) Then
                    Me.TextBox26.Text = reader.GetString(0)
                    Me.TextBox33.Focus()
                ElseIf X = 28 And TextBox33.Text <> reader.GetString(0) Then
                    Me.TextBox33.Text = reader.GetString(0)
                    Me.TextBox31.Focus()
                ElseIf X = 30 And TextBox31.Text <> reader.GetString(0) Then
                    Me.TextBox31.Text = reader.GetString(0)
                    Me.TextBox39.Focus()
                ElseIf X = 32 And TextBox39.Text <> reader.GetString(0) Then
                    Me.TextBox39.Text = reader.GetString(0)
                    Me.TextBox37.Focus()
                ElseIf X = 34 And TextBox39.Text <> reader.GetString(0) Then
                    Me.TextBox37.Text = reader.GetString(0)
                    Me.TextBox45.Focus()
                ElseIf X = 36 And TextBox45.Text <> reader.GetString(0) Then
                    Me.TextBox45.Text = reader.GetString(0)
                    Me.TextBox53.Focus()
                ElseIf X = 38 And TextBox53.Text <> reader.GetString(0) Then
                    Me.TextBox53.Text = reader.GetString(0)
                    Me.TextBox49.Focus()
                ElseIf X = 40 And TextBox49.Text <> reader.GetString(0) Then
                    Me.TextBox49.Text = reader.GetString(0)
                    Me.TextBox50.Focus()
                ElseIf X = 42 And TextBox50.Text <> reader.GetString(0) Then
                    Me.TextBox50.Text = reader.GetString(0)
                    Me.TextBox57.Focus()
                ElseIf X = 44 And TextBox57.Text <> reader.GetString(0) Then
                    Me.TextBox57.Text = reader.GetString(0)
                    Me.TextBox55.Focus()
                ElseIf X = 46 And TextBox55.Text <> reader.GetString(0) Then
                    Me.TextBox55.Text = reader.GetString(0)
                    Me.TextBox63.Focus()
                ElseIf X = 48 And TextBox63.Text <> reader.GetString(0) Then
                    Me.TextBox63.Text = reader.GetString(0)
                    Me.TextBox71.Focus()
                ElseIf X = 50 And TextBox71.Text <> reader.GetString(0) Then
                    Me.TextBox71.Text = reader.GetString(0)
                    Me.TextBox67.Focus()
                ElseIf X = 52 And TextBox67.Text <> reader.GetString(0) Then
                    Me.TextBox67.Text = reader.GetString(0)
                    Me.TextBox68.Focus()
                ElseIf X = 54 And TextBox68.Text <> reader.GetString(0) Then
                    Me.TextBox68.Text = reader.GetString(0)
                    Me.TextBox1.Focus()
                    Me.TextBox140.Focus()
                End If

                reader.Close()
            End If

        Next X
        Connection.Close() ' <-----------
        Me.TextBox138.Text = "Opdateret:  " & Now.ToShortTimeString

        ResumeLayout()
    End Sub
0
steensommer
Asked:
steensommer
2 Solutions
 
bcoppingCommented:
I would create a stored procedure to return all the bed numbers & HQV's. Then run through all the results with a while reader.read command assigning all the appropriate values.

Running 1 stored procedure rather than 23 should make it more efficient. rough code:

Dim Command As New SqlCommand("SELECT HCV, Plads FROM Sengeplads", Connection)
Dim reader As SqlDataReader = Command.ExecuteReader()
while reader.Read
   select case cint(reader(1))
      case  8
         TextBox1.text = cstr(reader(0))
         TextBox2.Focus()
   end select
end while
0
 
Bob LearnedCommented:
Where does the slowness occur?

Bob
0
 
steensommerAuthor Commented:
Hi

Is this what you mean:

  Sub Hent2()
        Dim Connection As New SqlConnection(Conn)
        Connection.Open()

        Dim Command As New SqlCommand("SELECT HCV, Plads FROM Sengeplads", Connection)
        Dim reader As SqlDataReader = Command.ExecuteReader()
        While reader.Read
            Select Case CInt(reader(1))
                Case 8
                    FrmPatientliste.TextBox1.Text = CStr(reader(0))
                    FrmPatientliste.TextBox2.Focus()
                Case 10
                    FrmPatientliste.TextBox2.Text = CStr(reader(0))
                    FrmPatientliste.TextBox4.Focus()
                Case 12
                    FrmPatientliste.TextBox4.Text = CStr(reader(0))
                    FrmPatientliste.TextBox3.Focus()
                Case 14
                    FrmPatientliste.TextBox3.Text = CStr(reader(0))
                    FrmPatientliste.TextBox9.Focus()
                Case 16
                    FrmPatientliste.TextBox9.Text = CStr(reader(0))
                    FrmPatientliste.TextBox12.Focus()
                Case 18
                    FrmPatientliste.TextBox12.Text = CStr(reader(0))
                    FrmPatientliste.TextBox21.Focus()
                Case 20
                    FrmPatientliste.TextBox21.Text = CStr(reader(0))
                    FrmPatientliste.TextBox29.Focus()
                Case 22
                    FrmPatientliste.TextBox29.Text = CStr(reader(0))
                    FrmPatientliste.TextBox25.Focus()
                Case 24
                    FrmPatientliste.TextBox25.Text = CStr(reader(0))
                    FrmPatientliste.TextBox26.Focus()
                Case 26
                    FrmPatientliste.TextBox26.Text = CStr(reader(0))
                    FrmPatientliste.TextBox33.Focus()
                Case 28
                    FrmPatientliste.TextBox33.Text = CStr(reader(0))
                    FrmPatientliste.TextBox31.Focus()
                Case 30
                    FrmPatientliste.TextBox31.Text = CStr(reader(0))
                    FrmPatientliste.TextBox39.Focus()
                Case 32
                    FrmPatientliste.TextBox39.Text = CStr(reader(0))
                    FrmPatientliste.TextBox37.Focus()
                Case 34
                    FrmPatientliste.TextBox37.Text = CStr(reader(0))
                    FrmPatientliste.TextBox45.Focus()
                Case 36
                    FrmPatientliste.TextBox45.Text = CStr(reader(0))
                    FrmPatientliste.TextBox53.Focus()
                Case 38
                    FrmPatientliste.TextBox53.Text = CStr(reader(0))
                    FrmPatientliste.TextBox49.Focus()
                Case 40
                    FrmPatientliste.TextBox49.Text = CStr(reader(0))
                    FrmPatientliste.TextBox50.Focus()
                Case 42
                    FrmPatientliste.TextBox50.Text = CStr(reader(0))
                    FrmPatientliste.TextBox57.Focus()
                Case 44
                    FrmPatientliste.TextBox57.Text = CStr(reader(0))
                    FrmPatientliste.TextBox55.Focus()
                Case 46
                    FrmPatientliste.TextBox55.Text = CStr(reader(0))
                    FrmPatientliste.TextBox63.Focus()
                Case 48
                    FrmPatientliste.TextBox63.Text = CStr(reader(0))
                    FrmPatientliste.TextBox71.Focus()
                Case 50
                    FrmPatientliste.TextBox71.Text = CStr(reader(0))
                    FrmPatientliste.TextBox67.Focus()
                Case 52
                    FrmPatientliste.TextBox67.Text = CStr(reader(0))
                    FrmPatientliste.TextBox68.Focus()
                Case 54
                    FrmPatientliste.TextBox68.Text = CStr(reader(0))
                    FrmPatientliste.TextBox1.Focus()
                    FrmPatientliste.TextBox140.Focus()

            End Select
        End While
    End Sub
0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
steensommerAuthor Commented:
Hi Bob

I think the slowness occurs because the code "selects" a textbox 24 times?

Steen
0
 
SanclerCommented:
>>
Is this what you mean
<<

I can't answer for bcopping, but it certainly looks like it.  

The .Focus lines are superfluous except for the last.  Each previous setting is just overriden by the next one.

An alternative approach would be to revert to getting the values one by one but modify the sql string so that a value is only returned if the value you are interested in has changed.  E.g.

"SELECT HCV FROM Sengeplads WHERE Plads = '" & X & "' AND HCV <> '" & Me.TextBox1.Text & "'"

If the reader returns nothing you just go on to the next call of it.  Effectively moving an "if" test into the command rather than getting a record over and then testing it.

But whether that would, in practice, be more efficient would depend on how often changes actually occur and in how many values.  And, so far as coding is concerned, if you did want to go down that road you should create your command once at the start of the sub; or even, as you are going to run the sub every 15 minutes, outside (but with scope for) the sub and just stick parameters in it for each different call.  

At the creation stage

Command.CommandText = "SELECT HCV FROM Sengeplads WHERE Plads = @Plads AND HCV <> @HCV"
Command.Parameters.Add(@Plads,SqlDbType.VarChar, 80)
Command.Parameters.Add(@HCV,SqlDbType.VarChar, 80)

At the calling stage

Command.Parameters(@Plads) = 8
Command.Parameters(@HCV) = TextBox1.Text

It may even be possible to put the data or the datasources for the parameters - in this case 8 and TextBox1.Text - in an array or hashtable and cycle through that but, although that might make the code look tidier, I am not sure that it would increase efficiency.

Roger
0
 
SanclerCommented:
Sorry, I see that

Command.Parameters(@Plads) = 8

should be

Command.Parameters(@Plads) = "8"

given that I set the parameter type as VarChar.

Roger
0
 
steensommerAuthor Commented:
Hi Sancler

Given that I'm a novice in VB .Net what code wood you prefer. The code in my last comment works fine but I can't telle if it is faster in real-time!
Steen
0
 
SanclerCommented:
It's very difficult to tell, out of context, which code is "best".  When (which is not very often) speed becomes an issue for anything I'm doing I generally test alternatives in (or with dummy data and conditions as close as possible to) the environment in which it is to be used.

Having said that, my feeling is that the code from your last post is better than that from your first.  Its upsides are that it is using a Select Case approach instead of the laborious If ... ElseIf approach and using one datareader rather then repeatedly creating new ones.  Its downsides are that it is bringing over data that you know you won't need (all the odd-numbered Plads records) and re-setting every textbox even if there's been no change in the relevant data.  But those look to me - at least in theory - trade-offs worth making.  

But whether seeking to go further than that would produce proportionate improvements, I am less sure.  What looked to me the biggest issues have already been addressed.  As I said previously, a factor is likely to be how much actually changes.  If 23 out of the 24 values changes every time then re-setting one textbox you don't really need to isn't losing much.  But if there's only one change every fourth time the sub is run the picture might alter.

So my approach would be not "can it be improved further?" but "is it good enough yet?"  If it works, and you and the users are sufficiently happy with it, I'd stick with the version from your second post (but get rid of all the .Focus lines except the last).  But if you really need further improvement then I reckon the alternative would be worth a try.  But it would be just that - a "try".  That is, I'd code it and test it and then try the two versions out to see which, in practice, was better.  My guess is that my alternative approach might be slightly faster.  But it is a guess and I also doubt that any difference would be sufficiently significant to be noticeable so far as users are concerned.

Roger
0
 
steensommerAuthor Commented:
Hi
I think I will stick with the last code BUT I can't remove the:  .focus because I have placed some code in:  textbook.leave

Thanks for your fast and very usefull help :0)

Regards Steen
0
 
steensommerAuthor Commented:
...thank to both of you  - Sorry
0
 
SanclerCommented:
Steen

I know the question is closed, and thanks for the points, but

>>
BUT I can't remove the:  .focus because I have placed some code in:  textbook.leave
<<

That could be a quite significant factor so far as performance goes.  I said above "If 23 out of the 24 values changes every time then re-setting one textbox you don't really need to isn't losing much".  I didn't say - but I was also thinking - and re-setting 24 textboxes if only one really needs it is not going to make a human- noticeable difference.  But if what's involved is not just re-setting a textbox but also firing other code then - depending what that other code is - it might affect the equation considerably.

Just a thought.

Roger
0
 
steensommerAuthor Commented:
Hmm  You are propably wright. The following code is fired for every 24 textboxes:

  Private Sub TextBox1_Leave(ByVal sender As Object, ByVal e As System.EventArgs) Handles TextBox1.Leave
        Check(TextBox1)
        Dim Connection As New SqlConnection(Conn)
        Connection.Open()
        Dim PatientID As String = TextBox1.Text
        If Microsoft.VisualBasic.Left(PatientID, 1) = "P" Then
            Dim Command As New SqlCommand("SELECT [Patient nr], [CPR nr], Fornavn, Efternavn, Kommunekoder, Amt, [Operation type], Tekst  FROM [PCI og KAG] WHERE [Patient nr] = '" & PatientID & "'", Connection)
            Dim reader As SqlDataReader = Command.ExecuteReader()
            If reader.Read() Then
                TextBox1.Text = reader.GetString(0)
                TextBox5.Text = reader.GetString(2) & " " & reader.GetString(3)
                If Not reader.IsDBNull(1) Then
                    TextBox73.Text = reader.GetString(1)
                Else
                    TextBox73.Text = ""
                End If
                If Not reader.IsDBNull(6) Then
                    TextBox122.Text = reader.GetString(6)
                Else
                    TextBox122.Text = ""
                End If
                If Not reader.IsDBNull(5) Then
                    TextBox126.Text = reader.GetString(5)
                Else
                    TextBox126.Text = ""
                End If
                If Not reader.IsDBNull(4) Then
                    TextBox130.Text = reader.GetString(4)
                Else
                    TextBox130.Text = ""
                End If
                If Not reader.IsDBNull(7) Then
                    TextBox134.Text = reader.GetString(7)
                Else
                    TextBox134.Text = ""
                End If

                reader.Close()
                Connection.Close()
            End If
        ElseIf Microsoft.VisualBasic.Left(PatientID, 1) = "R" Then
            Dim Command As New SqlCommand("SELECT [Patient nr], [CPR nr], Fornavn, Efternavn, Kommunekoder, Amt, [Operation type], Tekst  FROM RFA WHERE [Patient nr] = '" & PatientID & "'", Connection)
            Dim reader As SqlDataReader = Command.ExecuteReader()
            If reader.Read() Then
                TextBox1.Text = reader.GetString(0)
                TextBox5.Text = reader.GetString(2) & " " & reader.GetString(3)
                If Not reader.IsDBNull(1) Then
                    TextBox73.Text = reader.GetString(1)
                Else
                    TextBox73.Text = ""
                End If
                If Not reader.IsDBNull(6) Then
                    TextBox122.Text = reader.GetString(6)
                Else
                    TextBox122.Text = ""
                End If
                If Not reader.IsDBNull(5) Then
                    TextBox126.Text = reader.GetString(5)
                Else
                    TextBox126.Text = ""
                End If
                If Not reader.IsDBNull(4) Then
                    TextBox130.Text = reader.GetString(4)
                Else
                    TextBox130.Text = ""
                End If
                If Not reader.IsDBNull(7) Then
                    TextBox134.Text = reader.GetString(7)
                Else
                    TextBox134.Text = ""
                End If

                reader.Close()
                Connection.Close()
            End If
        ElseIf TextBox1.Text <> "" Then
            Dim Command As New SqlCommand("SELECT [Patient nr], [CPR nr], Fornavn, Efternavn, Kommunekoder, Amt, [Operation type], Tekst, [Operations dato]  FROM Kartotek WHERE [Patient nr] = '" & PatientID & "'", Connection)
            Dim reader As SqlDataReader = Command.ExecuteReader()
            If reader.Read() Then
                TextBox1.Text = reader.GetString(0)
                TextBox5.Text = reader.GetString(2) & " " & reader.GetString(3)
                If Not reader.IsDBNull(1) Then
                    TextBox73.Text = reader.GetString(1)
                Else
                    TextBox73.Text = ""
                End If
                If Not reader.IsDBNull(6) Then
                    TextBox122.Text = reader.GetString(6)
                Else
                    TextBox122.Text = ""
                End If
                If Not reader.IsDBNull(5) Then
                    TextBox126.Text = reader.GetString(5)
                Else
                    TextBox126.Text = ""
                End If
                If Not reader.IsDBNull(4) Then
                    TextBox130.Text = reader.GetString(4)
                Else
                    TextBox130.Text = ""
                End If
                If Not reader.IsDBNull(7) Then
                    TextBox134.Text = reader.GetString(7)
                Else
                    TextBox134.Text = ""
                End If
                If Not reader.IsDBNull(8) Then
                    If reader.GetDateTime(8) <> "01-01-1900" Then

                        TextBox118.Text = DateDiff("d", reader.GetDateTime(8), Today)
                    Else
                        TextBox118.Text = Nothing
                    End If
                Else
                    TextBox118.Text = Nothing
                End If

                reader.Close()
                Connection.Close()
            End If
        End If
    End Sub

    Private Sub TextBox1_TextChanged(ByVal sender As Object, ByVal e As System.EventArgs) Handles TextBox1.TextChanged
        Dim PatientID As String = TextBox1.Text
        Dim Connection As New SqlConnection(Conn)
        Connection.Open()
        Dim Command As New SqlCommand("SELECT [Patient nr] FROM Kartotek WHERE [Patient nr] = '" & PatientID & "'", Connection)
        Dim reader As SqlDataReader = Command.ExecuteReader()
        If reader.Read() Then
        Else
            TextBox5.Text = ""
            TextBox73.Text = ""
            TextBox118.Text = ""
            TextBox122.Text = ""
            TextBox126.Text = ""
            TextBox130.Text = ""
            TextBox134.Text = ""
        End If
        If Microsoft.VisualBasic.Left(PatientID, 1) = "P" Then
            TextBox1.BackColor = Color.LemonChiffon
            TextBox5.BackColor = Color.PaleGreen
            TextBox73.BackColor = Color.PaleGreen
            TextBox118.BackColor = Color.PaleGreen
            TextBox122.BackColor = Color.PaleGreen
            TextBox126.BackColor = Color.PaleGreen
            TextBox130.BackColor = Color.PaleGreen
            TextBox134.BackColor = Color.PaleGreen

        ElseIf Microsoft.VisualBasic.Left(PatientID, 1) = "R" Then
            TextBox1.BackColor = Color.LemonChiffon
            TextBox5.BackColor = Color.LightBlue
            TextBox73.BackColor = Color.LightBlue
            TextBox118.BackColor = Color.LightBlue
            TextBox122.BackColor = Color.LightBlue
            TextBox126.BackColor = Color.LightBlue
            TextBox130.BackColor = Color.LightBlue
            TextBox134.BackColor = Color.LightBlue

        ElseIf Microsoft.VisualBasic.Left(PatientID, 1) = "0" Or Microsoft.VisualBasic.Left(PatientID, 1) = "1" Or Microsoft.VisualBasic.Left(PatientID, 1) = "9" Or Microsoft.VisualBasic.Left(PatientID, 1) = "2" Or Microsoft.VisualBasic.Left(PatientID, 1) = "3" Then
            TextBox1.BackColor = Color.LemonChiffon

        Else
            TextBox1.Text = ""
            TextBox1.BackColor = Color.White
            TextBox5.BackColor = Color.White
            TextBox73.BackColor = Color.White
            TextBox118.BackColor = Color.White
            TextBox122.BackColor = Color.White
            TextBox126.BackColor = Color.White
            TextBox130.BackColor = Color.White
            TextBox134.BackColor = Color.White
            TextBox5.Text = ""
            TextBox73.Text = ""
            TextBox73.Text = ""
            TextBox118.Text = ""
            TextBox122.Text = ""
            TextBox126.Text = ""
            TextBox130.Text = ""
            TextBox134.Text = ""
        End If

    End Sub
0
 
SanclerCommented:
>>
it might affect the equation considerably.
<<

Do I get a prize for the understatement of the year? ;-)

1)  I think some of the code you've just posted could do with tidying/speeding up a bit anyway; and

2)  I also now think, in view of the fact that - even after tidying/speeding up - this code will take quite a bit more time to execute than just re-setting the textbox in the other sub would, you need to build back into the other sub some tests to see if the textbox really does need updating.

Roger
0
 
steensommerAuthor Commented:
OK - You are propably wright but then again ... it works! And some of the code can the reduced after changing the database.

...and Yes - you should get a prize   :0)

Steen
0

Featured Post

VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now