Insert contact information from form fields into MySQL database when button is clicked I am just a beginner trying to do my due diligence and research things properly. I am trying to go about the correct way of adding data into a database. I have read articles on the following:


Not showing ConnectionString in plaintext. I currently store it in App.Config and I am still trying to figure out how to encrypt it.
using keyword to deal with disposable items. 
catch try finally to deal with exceptions.
parameterized SQL statements to deal with SQL Injection.


I am looking for any advise to make my code better. I have revised it several times taking into account all the articles I have read about the topics above.

private void Btn_InsertData_Click(object sender, EventArgs e)
{
    if (TextboxesAreNullorEmpty() == true)
    {
        MessageBox.Show("Please fill out all fields!");
        return;
    }
    else
    {
        try
        {
            using (MySqlConnection conn = new MySqlConnection(Properties.Settings.Default.MySQLDBConnection))
            using (MySqlCommand cmd = new MySqlCommand())
            {
                conn.Open();
                cmd.Connection = conn;
                cmd.CommandText = "INSERT INTO `sites` (`ContractNumber`, `SiteName`, `SitePhoneNumber`, `SiteAddLine1`, `SiteAddLine2`, `SiteAddCity`, `SiteAddCounty`, `SiteAddPostcode`, `SiteAddCountry`) VALUES (@contract_name, @site_name, @site_phone_number, @site_add_line1, @site_add_line2, @site_add_city, @site_add_county, @site_add_postcode, @site_add_country)";
                cmd.Prepare();
                cmd.Parameters.AddWithValue("@contract_name", this.txtBox_ContractNumber.Text);
                cmd.Parameters.AddWithValue("@site_name", this.txtBox_SiteName.Text);
                cmd.Parameters.AddWithValue("@site_phone_number", this.txtBox_SitePhoneNumber.Text);
                cmd.Parameters.AddWithValue("@site_add_line1", this.txtBox_SiteAddLine1.Text);
                cmd.Parameters.AddWithValue("@site_add_line2", this.txtBox_SiteAddLine2.Text);
                cmd.Parameters.AddWithValue("@site_add_city", this.txtBox_SiteAddCity.Text);
                cmd.Parameters.AddWithValue("@site_add_county", this.txtBox_SiteAddCounty.Text);
                cmd.Parameters.AddWithValue("@site_add_postcode", this.txtBox_SiteAddPostcode.Text);
                cmd.Parameters.AddWithValue("@site_add_country", this.txtBox_SiteAddCountry.Text);
                cmd.ExecuteNonQuery();
                MessageBox.Show("Site Details Added");
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show(string.Format("An error occurred {0}", ex.Message), "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
        finally
        {
           ClearAllTextBoxes();
        }
    }

}
Not showing ConnectionString in plaintext. I currently store it in App.Config and I am still trying to figure out how to encrypt it.


Anything you can decrypt locally, a malevolent (and sufficiently skilled) user can too. More information given in the answer here.

There are other ways to avoid this:


Add a webservice backend layer. The frontend only connects to the backend (no connectionstring needed for that), and the backend has the connection string (which is out of reach for the frontend and its users).
Use Integrated Security, i.e. using the current Windows account for logging in to the database; as opposed to providing a username and password in the connection string. But you'd need to provide the needed rights to all of your users' Windows account, and then everyone is working under their own account (some companies do this, most don't).





  catch try finally to deal with exceptions.


Your current implementation seems okay, but I do wonder about the finally.

If there is no error, the textboxes get cleared. That seems okay. But if there is an error, the textboxes still get cleared? That doesn't seem user friendly. I'd expect you'd want to specifically keep the content of the textboxes so the user can retry saving the data.

try
{
    //save the data

    ClearAllTextBoxes();
}
catch (Exception ex)
{
    MessageBox.Show(string.Format("An error occurred {0}", ex.Message), "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}


If an exception occurs when saving the data; the exception will skip over the ClearAllTextBoxes(); and go straight to the catch. It will never empty the textboxes.
However, if no exception is encountered, then the code will continue with ClearAllTextBoxes(); and therefore clear the textboxes.


  Minor comment
  You may want to put a return; in your catch. Not necessary at the moment, but it will become relevant if there is more code after your try catch.




In general, it's not good to put data handling (the SQL query) inside the UI. You'd want to put at least one layer of separation between the two.

However, you did mention you're a beginner; so I'm not going to hammer this point right now. If this project is a simple test application; you don't have to implement that layer of separation.  

But I would advise trying to separate the two in future projects. You don't want a bad habit to form.



This is nitpicking, but 

if (TextboxesAreNullorEmpty() == true)


really grinds my gears. == true is redundant, you can simply use 

if (TextboxesAreNullorEmpty())




In order to reduce nesting, you can change your code. As it currently stands, you have:

if (TextboxesAreNullorEmpty())
{
     MessageBox.Show("Please fill out all fields!");
     return;
}
else
{
      //Do the work
}


You can omit the else.

if (TextboxesAreNullorEmpty())
{
     MessageBox.Show("Please fill out all fields!");
     return;
}

//Do the work


This works exactly the same way. The only difference is that you have less indentation in your code. While one extra level of indentation may not be an issue; a sufficiently complex algorithm can have several ìnstances of redundant indentation, at which point it does become a relevant consideration.

But since you are a beginner, this isn't really a big issue right now. Just keep it in mind for the future, to avoid a bad habit.



Although you're not yet at the point of abusing it; you should start looking into breaking up the code you have and putting it into multiple smaller methods.

Currently, you have a single method that does many different things:


Handle the button click event
Validate the form (this is already correctly abstracted into a separate method)
Set up the SQL connection and command
Update the data in the database
Handle errors


I would suggest you add two extra methods here:


Instead of putting the logic inside the click event handler, have the click event handler call a StartSave() method which does the work. The benefit of this becomes clear once you can have multiple triggers to save the data (since the act of saving data isn't uniquely connected to a single user control anymore).
Put the entire try catch into a separate method, e.g. UpdateDatabase(). Ideally, you'd want to use method parameters here for all the input variables; which will give you a layer of separation between your form controls and the SQL parameters.