0
Private Sub txt_sname_GotFocus(ByVal sender As Object, ByVal e As System.EventArgs) Handles txt_sname.GotFocus
        Dim fcs As String
        fcs = "select fname,dept from nstudent where stid = '" & txt_sid.Text & "'"
        scmd1 = New SqlCommand(fcs, con)
        dr1 = scmd1.ExecuteReader
        If dr1.HasRows Then
            Do While (dr1.Read)
                txt_sname.Text = dr1.Item(0)
                cmb_dept.Text = dr1.Item(1)
            Loop
        Else
            MsgBox("Not Found")
        End If
        scmd1.Dispose()
        If Not dr1.IsClosed Then dr1.Close()
End Sub

The above code for data from database and pass to textbox. When am running the program and checking with data which already present in database, it working properly. but checking with some other data(which not present in db)following error is occurring and exiting.

error: "There is already an open DataReader associated with this Command which must be closed first."

pls help me..

Hari
  • 137
  • 3
  • 5
  • 14
  • Accessing a database in a GotFocus event? well I don't have an answer, but you have choosen a very dangerous place to put such processing. (Not to mention the messagebox in the middle) – Steve Mar 17 '13 at 19:03
  • hav anyother way to display that msg? – Hari Mar 17 '13 at 19:17
  • I suppose, because you have not declared the DataReader (dr1) locally in this procedure, something in your code raises an exception or cause an abnormal exit from the event code. Then your datareader is never closed and the next call fails – Steve Mar 17 '13 at 21:07

1 Answers1

1

Some observations:

Instead of using a global command object, use a local one. Especially since you are creating a new command anyway. And it looks like this is true of the dr1 as well.

You are not preventing SQL injection, so someone can type text in txt_sid that causes security issues by deleting data, dropping tables, or getting access to other data in the database.

You are looping and setting the same variables multiple times. If there is only going to be one record, don't bother looping.

Wrap the entire thing around a try/catch

Private Sub txt_sname_GotFocus(ByVal sender As Object, ByVal e As System.EventArgs) Handles txt_sname.GotFocus
    Dim cmd1 As SqlCommand = New SqlCommand(fcs, "select fname,dept from nstudent where stid = @stid")
    cmd1.Parameters.Parameters.AddWithValue("@stid", txt_sid.Text)

    Dim studentReader as SqlDataReader

    Try
        studentReader = scmd1.ExecuteReader
        If studentReader.Read Then
            txt_sname.Text = studentReader.Item(0)
            cmb_dept.Text = studentReader.Item(1)
        Else
            MsgBox("Not Found")
        End If
    Finally
        studentReader.Close()
        cmd1.Dispose()
    End Try
End Sub

Finally, I think you might want to actually do this when txt_sid changes, not when txt_sname gets focus.

scott.korin
  • 2,537
  • 2
  • 23
  • 36
  • +1, although I would put both the `SqlCommand` and the `SqlDataReader` in a `Using` block: `Using cmd1 As New SqlCommand(...)`, and `Using dr1 As SqlDataReader = cmd1.ExecuteReader(...)`. This, and if the connection object is properly opened and closed (the OP's code doesn't suggest this), the OP's error should disappear. – stakx - no longer contributing Mar 17 '13 at 21:19
  • I agree with the Using statement. I was considering going that way but in the end I didn't. – scott.korin Mar 17 '13 at 21:27