First off, you should never hard code your database connection string. Instead define your connection string in the
web.config
and reference the value from there. See:
Creating a Connection String and Working with SQL Server LocalDB | Microsoft Docs[
^]
For example:
<connectionStrings>
<add name="MyDBConnectionString" connectionString="Data Source=.\\SQLEXPRESS;Database=sample3;Integrated Security=true" providerName="System.Data.SqlClient" />
</connectionStrings>
You can then reference your Connection String value via
ConfigurationManager
class like this:
string dbConnectionString = ConfigurationManager.ConnectionStrings["MyDBConnectionString"].ConnectionString;
Second, I've seen that you have appended the value from your
DropDownList
directly to your SQL query which can potentially leads you to SQL Injection. You should never append the values directly to your SQL query even if the value came from a
DropDownList
. Instead, use SQL paramterize query always. See:
Protect Your Data: Prevent SQL Injection[
^]
Here's an example:
private void PopulateForm(int id){
using (SqlConnection connection = new SqlConnection(GetDBConnectionString())) {
using (SqlCommand cmd = new SqlCommand("SELECT * FROM test WHERE Id= @Id", connection)) {
connection.Open();
cmd.CommandType = CommandType.Text;
cmd.Parameters.AddWithValue("@Id", id);
SqlDataReader rdr = cmd.ExecuteReader();
while (rdr.Read())
{
txtName.Text = dr["Name"].ToString();
txtMobile.Text = dr["Mobile"].ToString();
txtEmail.Text = dr["Email"].ToString();
}
}
}
}
protected void ddlId_SelectedIndexChanged(object sender, EventArgs e){
PopulateForm(Convert.ToInt32(ddlId.SelectedValue));
}
Third, you should initialize your SQL connection when doing a database operation such as Update and Delete. In your code, it seems like you were reusing/referencing the variable
con
directly within your Update and Delete button event which that event will never be triggered, and your SQL connection object is no longer being initialized.
Fourth, make it a habit to put objects that eat resources such as
SqlConnection
,
SqlCommand
within a
using block
to ensure that objects will be properly disposed and after they are used.
Finally, I would recommend you to separate your code/logic for updating /deleting data to database and keep them out from your
Button
click event for the ease of maintenance and separation of concerns.
Your Update and Delete code should now be something like this:
private string GetDBConnectionString(){
return ConfigurationManager.ConnectionStrings["MyDBConnectionString"].ConnectionString;
}
private void UpdateRecord(int id, string name, string email, string mobile){
using (SqlConnection connection = new SqlConnection(GetDBConnectionString())) {
using (SqlCommand cmd = new SqlCommand("YourUpdateStoredProcedureName", connection)) {
connection.Open();
cmd.CommandType = CommandType.StoredProcedure;
cmd.Parameters.AddWithValue("@Name", name);
cmd.Parameters.AddWithValue("@Mobile", mobile);
cmd.Parameters.AddWithValue("@Email", email);
cmd.Parameters.AddWithValue("@Id", id);
cmd.ExecuteNonQuery();
}
}
}
private void DeleteRecord(int id){
using (SqlConnection connection = new SqlConnection(GetDBConnectionString())) {
using (SqlCommand cmd = new SqlCommand("YourDeleteStoredProcedureName", connection)) {
connection.Open();
cmd.CommandType = CommandType.StoredProcedure;
cmd.Parameters.AddWithValue("@Id", id);
cmd.ExecuteNonQuery();
}
}
}
You can then call those methods something like this:
protected void btnUpdate_Click(object sender, EventArgs e){
UpdateRecord(Convert.ToInt32(ddlId.SelectedValue), txtName.Text, txtEmail.Text, txtMobile.Text);
}
protected void btnUpdate_Click(object sender, EventArgs e){
DeleteRecord(Convert.ToInt32(ddlId.SelectedValue));
}