-3

When trying to register a new account in my program, via the LocalDB, I get an error stating:

ExecuteNonQuery requires an open and available Connection. The connection's current state is closed

But as you can see, I am opening the connection already

I have already tried setting the connection string in multiple places but to no avail


// The  methods which will be called if the user tries to log in or register
        private void LoginRegisterClick(object sender, EventArgs e)
        {
            // The following lines of code will execute if the user tries to register
            if (lblView.Text == "Register")
            {
                // If all the textboxes have passed validation, the details from the textboxes are retrieved and stored
                if (ucRegister1.AllFieldsValidated() == true)
                {
                    string[] details = ucRegister1.ReturnRegisterDetails();
                    // (cmdCountUsernames) Checks to see if new username is unique
                    Helper.ConnectionToDB().Open();
                    SqlCommand cmdCU = new SqlCommand(@"SELECT COUNT(*) FROM LoginsTable WHERE Login = '" + details[6] + "'", Helper.ConnectionToDB());
                    try
                    {
                        int count = (int)cmdCU.ExecuteScalar();
                        //int count = Convert.ToInt32(cmdCU.ExecuteScalar());
                        // If the new username is unique, the record is added into MainParentTable
                        if (count == 0)
                        {
                            try
                            {
                                // Order of the details:         
                                // details[(0)Firstname, (1)Surname, (2)DOB, (3)HomeTel, (4)MobileTel, (5)Address, (6)Username, (7)Password, (8)Email]

                                // (cmdCNL) Creates a new record in the LoginsTable
                                SqlCommand cmdCNL = new SqlCommand(@"INSERT INTO LoginsTable (Login, Password) VALUES (@login, @password)", Helper.ConnectionToDB());
                                Helper.ConnectionToDB().Open();
                                cmdCNL.Parameters.AddWithValue("@login", details[6]);
                                cmdCNL.Parameters.AddWithValue("@password", details[7]);
                                cmdCNL.ExecuteNonQuery();

                                // (cmdFindUserID) Finds the UserID of the new acccount which was just created above.
                                SqlCommand cmdFUID = new SqlCommand(@"SELECT * FROM LoginsTable WHERE Login = @login", Helper.ConnectionToDB());
                                cmdFUID.Parameters.AddWithValue("@login", details[6]);
                                var da = new SqlDataAdapter(cmdFUID);
                                var dt = new DataTable();
                                da.Fill(dt);
                                globalVariables.userIDSelected = dt.Rows[0]["UserID"].ToString();

                                // (cmdInsertMainParentTable) Adds to the MainParentTable, a new parent using the UserID from the code above.
                                SqlCommand cmdIMPT = new SqlCommand(@"INSERT INTO MainParentTable (UserID, FirstName, Surname, DOB, Address, HomeTelephone, MobileTelephone, Email) VALUES (@userid, @firstname, @surname, @dob, @address, @hometelephone, @mobiletelephone, @email)", globalVariables.conDB);
                                cmdIMPT.Parameters.AddWithValue("@userid", globalVariables.userIDSelected);
                                cmdIMPT.Parameters.AddWithValue("@firstname", details[0]);
                                cmdIMPT.Parameters.AddWithValue("@surname", details[1]);
                                cmdIMPT.Parameters.AddWithValue("@dob", Convert.ToDateTime(details[2]));
                                cmdIMPT.Parameters.AddWithValue("@address", details[5]);
                                cmdIMPT.Parameters.AddWithValue("@hometelephone", details[3]);
                                cmdIMPT.Parameters.AddWithValue("@mobiletelephone", details[4]);
                                cmdIMPT.Parameters.AddWithValue("@email", details[8]);
                                cmdIMPT.ExecuteNonQuery();
                                Helper.ConnectionToDB().Close();
                            }
                            catch (Exception msg2)
                            {
                                MessageBox.Show("Error Message 2: " + msg2);
                            }
                        }
                    }
                    catch (Exception msg)
                    {
                        MessageBox.Show("Error Message: " + msg);
                        throw;
                    }
                   // Helper.ConnectionToDB().Close();



                    // Populates and brings ucLogIn to the front of the form
                    ucLogIn1.PopulateLogins(details[6], details[7]);
                    lblView.Text = "Log In";
                    lblView.Location = new Point(293, 30);
                    ucLogIn1.BringToFront();                    
                }
                return;      
            }

Count should be set to either 0 or 1 based on the result of the query


GlobalVariables.Con simply does this

public static SqlConnection conDB = Helper.ConnectionToDB();

My helper class is as follows:

public static class Helper
{
    public static SqlConnection ConnectionToDB()
    {
        return new SqlConnection (@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename = G:\myDatabase.mdf; Integrated Security = True"); }
    } 
}
sticky bit
  • 36,626
  • 12
  • 31
  • 42
Mr F
  • 33
  • 1
  • 1
  • 5
  • Can you show the code of `Helper.ConnectionToDB()`? And, unrelated but BTW, `AddWithValue()` is [evil](http://www.dbdelta.com/addwithvalue-is-evil/). – sticky bit Apr 20 '19 at 15:51
  • It fails on the line int count = (int)cmdCU.ExecuteScalar(); – Mr F Apr 20 '19 at 15:56
  • GlobalVariables.Con simply does this ====> public static SqlConnection conDB = Helper.ConnectionToDB(); My helper class is as follows: public static class Helper { public static SqlConnection ConnectionToDB() { return new SqlConnection (@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename = G:\myDatabase.mdf; Integrated Security = True"); } } – Mr F Apr 20 '19 at 15:58
  • Connections should be created, used and disposed of in the smallest scope possible - typically a `using` block. Same goes for COmmand and DataReader objects. That makes that helper more trouble than it is saves - the data all saved as string is another Big Red Flag. Also, passwords should be salted and hashed before saving to the database – Ňɏssa Pøngjǣrdenlarp Apr 20 '19 at 16:03
  • You should have [edit] your question to add the additional information not the comments. This time I did this for you. But the next time please remember and do it yourself. – sticky bit Apr 20 '19 at 16:03
  • Thank you @Stickybit First time asking on StackOverflow – Mr F Apr 20 '19 at 16:08

1 Answers1

2

The problem is caused by this line (and similar lines in following commands)

SqlCommand cmdCU = new SqlCommand(........, Helper.ConnectionToDB());

I bet that your Helper.ConnectionToDB creates new instances of the SqlConnection object every time you call it.

Now, even if you have this line

  Helper.ConnectionToDB().Open();

You are opening a connection instance but, because the command calls again Helper.ConnectionToDB, you are getting a different instance in each command and using it while still closed.

You need a total different approach

.... previous stuff....
if (ucRegister1.AllFieldsValidated() == true)
{
    string[] details = ucRegister1.ReturnRegisterDetails();
    using(SqlConnection cnn = Helper.ConnectionToDB())
    {
        cnn.Open();
        SqlCommand cmdCU = new SqlCommand("......", cnn);

        .... replace all calls to Helper.ConnectionDB with the cnn variable ....

        .....
    }  // <== This close the connection 
}

The using statement helps you here to keep under control your SqlConnection and the resources it uses. SqlConnection is a disposable object with important unmanaged resources linked to it. It is essential to free these resources as soon as possible and the using statement guarantees that when the code exits the using block the SqlConnection will be closed and disposed freeing the resources.

Steve
  • 213,761
  • 22
  • 232
  • 286
  • Thanks for this @Steve. I have followed this until line 8 of your instructions. When I add 'cnn' after the SQL statement on that line, it states an error "The name cnn does not exist in the current context" – Mr F Apr 20 '19 at 16:13
  • Should not be possible. Review my code, the variable cnn is in context at the SqlCommand line. Check if there are some stray semi-colon that could change the scope – Steve Apr 20 '19 at 16:52