ConnectionString property. The connection's current state

Question

Here is the current coding for my login form

 Option Strict On
'-------------------------------------------
' Imports required for DB Connectivity
'------------------------------------------
Imports System.Data.OleDb
Imports ShadowLogin.GlobalVariables

Public Class ShadowLogin
Dim Main As New ShadowMain

'-------------------------------------------------------------------------------------------------------------------------------
' Establish Database Connectivity parameters
'-------------------------------------------------------------------------------------------------------------------------------
Dim cnnOLEDB As New OleDbConnection
Dim cmdOLEDB As New OleDbCommand
Dim strConnectionString As String = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & Environment.CurrentDirectory & "\shadow.mdb"


'-------------------------------------------------------------------------------------------
' System Login
'-------------------------------------------------------------------------------------------
Private Sub btnLogin_Click(sender As Object, e As EventArgs) Handles btnLogin.Click

    Dim selectedbutton As New RadioButton
    Dim Cancel As Boolean

    'Checks RedID is numerical digits only 
    If Not IsNumeric(txtRedID.Text) Then
        MsgBox("Please Check RedID.", vbInformation)

        'you may also consider erasing it
        txtRedID.Text = ""
        txtRedID.Focus()
        Exit Sub

        'checks that RedID is 9 characters long
    ElseIf txtRedID.Text.Length < 9 Then
        txtRedID.Focus()
        txtRedID.SelectAll()
        MsgBox("Please Check RedID", MsgBoxStyle.Exclamation)
        Exit Sub
    Else
        Cancel = True
    End If

    'Checks that a password has been entered
    If txtPassword.Text = "" Then
        MsgBox("Please Enter a Password", MsgBoxStyle.Exclamation, MsgBoxStyle.OkOnly)
        Exit Sub
    End If

    'if radiobutton is selected then continue
    If GroupBox1.SelectedRadioButton(selectedbutton) Then
        Main = New ShadowMain
        'Checks if radiobutton is selected and hides tabs 
        If optStudent.Checked = True Then
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage2)
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage3)

        ElseIf optFaculty.Checked = True Then
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage1)
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage3)

        ElseIf optDepartmentChair.Checked = True Then
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage1)
            ShadowMain.TabControl1.TabPages.Remove(ShadowMain.TabPage2)

        End If
    Else

        'if no radiobutton is selected display messagebox
        MsgBox("Please select a role")
        Exit Sub
    End If

    ' Check which role is selected and proceed accordingly


    '--------------------------------------------------------------------------------
    ' Student Role Login
    '--------------------------------------------------------------------------------
    If optStudent.Checked Then 'If user select a student role


        ' Let's Search for the User and Password

        cnnOLEDB.ConnectionString = strConnectionString
        cnnOLEDB.Open()
        ' Select the Student Record base on User Input
        cmdOLEDB.CommandText = "SELECT * FROM [Student] WHERE [SRedID]=" & txtRedID.Text
        cmdOLEDB.Connection = cnnOLEDB
        Dim rdrOLEDB As OleDbDataReader = cmdOLEDB.ExecuteReader

        'If we can find a match..
        If rdrOLEDB.Read = True Then
            ' See that the User, Password and Active status okay
            If rdrOLEDB.Item(0).ToString = txtRedID.Text And rdrOLEDB.Item(9).ToString = txtPassword.Text And rdrOLEDB.Item(13).ToString = "True" Then
                'Populate global userID (for future needs)
                userID = txtRedID.Text()
                'Populate a role for (for future needs)
                role = "Student"
                'Cleanup the login form
                Call LoginCleanup()
                ' Populate personal data in the Student Home
                ShadowMain.lblLName.Text = rdrOLEDB.Item(1).ToString
                ShadowMain.lblFName.Text = rdrOLEDB.Item(3).ToString
                ShadowMain.lblMIn.Text = rdrOLEDB.Item(2).ToString
                ShadowMain.lblAddOne.Text = rdrOLEDB.Item(5).ToString
                ShadowMain.lblCity.Text = rdrOLEDB.Item(6).ToString
                ShadowMain.lblState.Text = rdrOLEDB.Item(7).ToString
                ShadowMain.lblZip.Text = rdrOLEDB.Item(8).ToString
                ShadowMain.lblPhone.Text = rdrOLEDB.Item(10).ToString
                ShadowMain.lblEmail.Text = rdrOLEDB.Item(15).ToString
                'Show student home
                ShadowMain.Show()
                ' Close table and database connection (required) 
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            Else
                'If user / password don't match
                MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
                ' clear login window
                Call ClearLogin()
                ' Close table and database connection (required)
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            End If
        Else
            ' Close table and database connection (required)
            rdrOLEDB.Close()
            cnnOLEDB.Close()
            'If user / password don't match
            MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
            Call ClearLogin()
        End If


        '--------------------------------------------------------------------------------
        ' Faculty Role Login
        '--------------------------------------------------------------------------------

    ElseIf optFaculty.Checked Then

        ' Let's Search for the User and Password
        cnnOLEDB.Close()
        cnnOLEDB.ConnectionString = strConnectionString
        cnnOLEDB.Open()
        cmdOLEDB.CommandText = "SELECT * FROM [Faculty] WHERE [FRedID]=" & txtRedID.Text
        cmdOLEDB.Connection = cnnOLEDB
        Dim rdrOLEDB As OleDbDataReader = cmdOLEDB.ExecuteReader


        'If we can find a match.
        If rdrOLEDB.Read = True Then
            If rdrOLEDB.Item(0).ToString = txtRedID.Text And rdrOLEDB.Item(5).ToString = txtPassword.Text And rdrOLEDB.Item(9).ToString = "True" Then
                userID = txtRedID.Text()
                role = "Faculty"
                Call LoginCleanup()
                ShadowMain.lblLastNameFac.Text = rdrOLEDB.Item(2).ToString
                ShadowMain.lblFirstNameFac.Text = rdrOLEDB.Item(3).ToString
                ShadowMain.lblMiddleIntFac.Text = rdrOLEDB.Item(4).ToString
                ShadowMain.lblOfficeFac.Text = rdrOLEDB.Item(8).ToString
                ShadowMain.lblPhoneFac.Text = rdrOLEDB.Item(6).ToString
                ShadowMain.lblEmailFacl.Text = rdrOLEDB.Item(7).ToString
                ShadowMain.Show()
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            Else
                MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
                Call ClearLogin()
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            End If
        Else
            rdrOLEDB.Close()
            MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
            Call ClearLogin()
        End If



        '--------------------------------------------------------------------------------
        ' Department Chair Role Login
        '--------------------------------------------------------------------------------

    ElseIf optDepartmentChair.Checked Then
        ' Let's Search for the User and Password
        Dim rdrOLEDB As OleDbDataReader = cmdOLEDB.ExecuteReader
        cnnOLEDB.Close()
        cnnOLEDB.ConnectionString = strConnectionString
        cnnOLEDB.Open()
        cmdOLEDB.CommandText = "SELECT [DCRedID,Password] FROM [DepartmentChair] WHERE [DCRedID]=" & txtRedID.Text
        cmdOLEDB.Connection = cnnOLEDB


        'If we can find a match..
        If rdrOLEDB.Read = True Then
            If rdrOLEDB.Item(0).ToString = txtRedID.Text And rdrOLEDB.Item(1).ToString = txtPassword.Text Then
                userID = txtRedID.Text()
                role = "Department Chair"
                Call LoginCleanup()
                ShadowMain.Show()
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            Else
                MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
                Call ClearLogin()
                rdrOLEDB.Close()
                cnnOLEDB.Close()
            End If
        Else
            rdrOLEDB.Close()
            cnnOLEDB.Close()
            MsgBox("Sorry, username or password not found", MsgBoxStyle.OkOnly, "Invalid Login")
            Call ClearLogin()
        End If
    End If


End Sub

The issue that i am having right now is if i fail to login the first time and then try to login a second time i get the error:

Not allowed to change the 'ConnectionString' property. The connection's current state is open.

I looked up and found a solution saying i need to make sure my connection is closed before hand, and it solved that issue but now i get this issue:

Invalid attempt to call Read when reader is closed.

I tried to label everything as best as possible, and added in the majority of my code since people always ask to see it. Any help is appreciated on this.

Dont use a global connection. Create it, use it and dispose of it for each use. See [this answer](http://stackoverflow.com/q/28213871/1070452) Pretty much the same for OleDbCommand objects. — Ňɏssa Pøngjǣrdenlarp, Nov 27 '15 at 22:09
...you should also be using SQL Parameters to avoid injection, and passwords should never be stored as plaintext, but should be hashed. — Ňɏssa Pøngjǣrdenlarp, Nov 27 '15 at 22:24
Thank you, ill look up the hashed password, never heard of that before. Ill also check out that post as well — James Swan, Nov 27 '15 at 22:30
See [Is it safe for me to store usernames and passwords in the database?](http://stackoverflow.com/q/31146658/1070452) One of the answers has the compleat code for hasing PWs in VB — Ňɏssa Pøngjǣrdenlarp, Nov 27 '15 at 22:37
thats awesome, ill def look into using that. Appreciate the help — James Swan, Nov 27 '15 at 22:42
@Plutonix - *"Dont use a global connection. Create it, use it and dispose of it for each use."* - That is not always the best approach when working with an Access database. See my comment to Steve's answer below for details. — Gord Thompson, Nov 28 '15 at 17:30

Steve · Accepted Answer · 2015-11-28T14:19:07.207

I really suggest you to split your enormous block of code in smaller parts. Actually this code is very difficult to read and to debug. To control the state of your connection you are forced to introduce spurious calls to Connection.Close and DataReader.Close but, as you can see, this is very error prone because you cannot be sure to have covered every code path.

Instead if you introduce smaller blocks of code you have a better control on when open and close the connection with the associated reader.

For example

If optStudent.Checked Then 'If user select a student role
    '--------------------------------------------------------------------------------
    ' Student Role Login
    '--------------------------------------------------------------------------------
   HandleStudentRole()
ElseIf optFaculty.Checked Then
    '--------------------------------------------------------------------------------
    ' Faculty Role Login
    '--------------------------------------------------------------------------------
     HandleFacultyRole()

ElseIf optDepartmentChair.Checked Then
    '--------------------------------------------------------------------------------
    ' Department Chair Role Login
    '--------------------------------------------------------------------------------
    HandleDepartmentRole()
End If

Now you could write the smaller blocks of code making sure that every block opens and closes the connection and the reader (Here I will show the block for the student role, but the other are similar)

Private Sub HandleStudenRole()
  ' Let's Search for the User and Password

    Using cnnOLEDB = new OleDbConnection(strConnectionString)
    Using cmdOleDB = new OleDbCommand("SELECT * FROM [Student] WHERE [SRedID]= @studentID", cnnOleDB)
       cnnOLEDB.Open()
       ' Select the Student Record base on User Input
       cmdOleDb.Parameters.Add("@studentID", OleDbType.VarWChar).Value = Convert.ToInt32(txtRedID.Text)
       Using rdrOLEDB = cmdOLEDB.ExecuteReader
           'If we can find a match..
          If rdrOLEDB.Read = True Then
             ......
          Else
            'If user / password don't match
            MsgBox(....)
            ' clear login window
            Call ClearLogin()
          End If
       End Using
    End Using
    End Using
End Sub

I have used the Using statement that allows removing all the code that you have written to close the connection and the reader. Using will ensure that the Close and Dispose methods are called whenever the flow of code exits from the using block also if you get an exception. Of course the query is parameterized (albeit in this scenario is very difficult to exploit an Sql Injection)

Notice also, that in my HandleStudentRole block the variables cnnOLEDB and cmdOLEDB are local variables. You don't need anymore to declare them at the global scope. There is no drop in performance if you declere and initialize them everytime you need them and the cleanliness of your code is far better.

*"no drop in performance if you declere and initialize [connections] everytime you need them"* - That is less certain with Access than it is with server-based databases. Remember that Access is a file-based DBMS so it must be much more "intimate" with the database file, which can make a difference when the database file is on a network share. Example: VB.NET test app doing 2 SELECTs on a 25MB .mdb with 2000 tables. Keeping the connection open for both SELECTs generated 429KB total network traffic. Closing and re-opening the connection between SELECTs increased the traffic to 711KB (66% more). — Gord Thompson, Nov 28 '15 at 17:10
Uhm, yes probably, in this instance, you are correct. Nevertheless I feel that trading off a bit of performances to avoid the housekeeping hassle (not to mention the maintain a correct habit to work in a more standard way) is a thing to consider. In any case the real answer here is _measure_ — Steve, Nov 28 '15 at 19:14
@JamesSwan this answer provides a lot of good advice to solve the problem *and* make your code more readable and organized. If it helped, you should click the checkmark next to it. Later when you get to 15, you can upvote questions and answers (anywhere/everywhere) which you find helpful or enlightening — Ňɏssa Pøngjǣrdenlarp, Dec 07 '15 at 16:11
just realized what that check mark was for about 10mins ago. — James Swan, Dec 07 '15 at 16:29

ConnectionString property. The connection's current state

1 Answers1