Should I be using a stored procedure for this function in my code behind?

We typically do our saves and deletes for our asp.net pages (SQL Server as back-end) in the code behind. I've seen some reference the idea of creating a stored procedure for this purpose. Is this the best approach or is what I'm doing practical enough? Any suggestions? Here's an example of the delete button function (first half is for triggers set up in SQL):

protected void DeleteClick(object sender, EventArgs e)
{
    cookie = Request.Cookies["pAuthCookie"];
    if (txtKey.Text == "")
    {
        string scriptstring = "alert('User Error - Must Select Fromn the LIST first.');";
        ScriptManager.RegisterStartupScript(Tab1, typeof(string), "alertscript", scriptstring, true);
        if (TabContainer1.ActiveTabIndex == 1)
            TabContainer1.ActiveTabIndex = 0;
        return;
    }
    else
    {
        String fprescript = txtKey.Text;
        string empno = cookie["thenum"].ToString();

        string sqlstring1 = "UPDATE prescriptions set empfk = @empfk WHERE prescriptpk = @prescriptpk";
        con.Open();
        SqlCommand cmd1 = new SqlCommand(sqlstring1, con);
        cmd1.Parameters.Add("@prescriptpk", SqlDbType.Int);
        cmd1.Parameters.Add("@empfk", SqlDbType.Int);
        cmd1.Parameters["@prescriptpk"].Value = Convert.ToInt32(fprescript);
        cmd1.Parameters["@empfk"].Value = empno.Trim();
        cmd1.ExecuteNonQuery();

        con.Close();
        string sqlstring = "DELETE FROM prescriptions WHERE prescriptpk = @prescriptpk";
        con.Open();
        SqlCommand cmd = new SqlCommand(sqlstring, con);
        cmd.Parameters.Add("@prescriptpk", SqlDbType.Int);
        cmd.Parameters["@prescriptpk"].Value = Convert.ToInt32(fprescript);
        cmd.ExecuteNonQuery();

        con.Close();
        clearoutdata();
        TabContainer1.ActiveTabIndex = 0;
        GridView1.DataBind();
    }
}

This is overall comments on above code

1) Encapsulate what is repeating

In your method you are opening sql connection, adding parameter and executing it one by one. Rather doing this please extract out it into one method.

private void ExecuteNonQuery(string connectionstring, string query, IEnumerable parameters =null)
  {
            using (var connection = new SqlConnection(connectionstring))
            {
                connection.Open();
                using (var command = new SqlCommand(query, connection))
                {
                    if (parameters != null)
                    {
                        foreach (var parameter in parameters)
                        {
                            command.Parameters.Add(parameter);
                        }
                    }
                    command.ExecuteNonQuery();
                }
            }
   }

2) Always dispose the connection

In above code your opening connection and closing it, but this can lead into connection leak in case of errors. Please wrap your code into using statement or try clause like this

using (var connection = new SqlConnection())
{

}

3) Distribute your responsibility into layer

The above code is doing too much

• script registering

• Sql Connection stuff

• Data Populating and clearing

Please separate out your code into at least two classes
1) Fetching data class
2) UI binding stuff class

4)inline SQL vs Stored Procedure

Stored Procedure is good as your code lies at one place in db but creating Stored Procedure for one line of SQL is not a good idea. If your logic is going to be complex and big ,Use a Stored procedure other wise inline sql.

PS: I have created a gist named SQLHelper class for you to abstract out the sql connection and command things,

https://gist.github.com/paritoshmmmec/5c3855af7215ac87d787

StackExchange Code Review Q#108077, answer score: 2