Want to protect your cyber security and still get fast solutions? Ask a secure question today.Go Premium

x
?
Solved

Please Overlook Code that performs Insert to DB

Posted on 2011-10-26
9
Medium Priority
?
237 Views
Last Modified: 2012-05-12
Hello Experts,

I was wondering if someone could possible overlook my code and tell me if this is a good coding practice for Inserting values into a DB.

I also need to email the user who fills the form out a confirmation email but before I add that part in I would like to have just the insert part looked over.

Thanks in advance!!!
protected void btn_NewUser_Click(object sender, EventArgs e)
    {
        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_InsertUsers";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.AddWithValue("@c_id", SqlDbType.Int).Value = ddlClient.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@bldg_id", SqlDbType.Int).Value = ddlBldgLocation.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@s_id", SqlDbType.Int).Value = ddlState.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@users_flname", SqlDbType.VarChar).Value = txtName.Text;
            cmd.Parameters.AddWithValue("@users_street_address", SqlDbType.VarChar).Value = txtHomeAddress.Text;
            cmd.Parameters.AddWithValue("@users_city", SqlDbType.VarChar).Value = txtCity.Text;
            cmd.Parameters.AddWithValue("@users_zip", SqlDbType.VarChar).Value = txtZipCode.Text;
            cmd.Parameters.AddWithValue("@users_phone", SqlDbType.VarChar).Value = txtHomePhone.Text;
            cmd.Parameters.AddWithValue("@users_work_ext", SqlDbType.VarChar).Value = txtWorkExtension.Text;
            cmd.Parameters.AddWithValue("@users_email", SqlDbType.VarChar).Value = txtEmailAddress.Text;
            cmd.Parameters.AddWithValue("@users_username", SqlDbType.VarChar).Value = txtUserName.Text;
            cmd.Parameters.AddWithValue("@users_password", SqlDbType.VarChar).Value = txtPassword.Text;

            try
            {
                conn.Open();

                cmd.ExecuteNonQuery();

                Response.Redirect("newuser_success.aspx");
            }

            catch (Exception ex)
            {
                lblInsertError.Text = ex.Message.ToString();
            }

            finally
            {
                conn.Close();
            }
        }
    }

Open in new window

0
Comment
Question by:asp_net2
  • 5
  • 4
9 Comments
 
LVL 35

Expert Comment

by:Paul MacDonald
ID: 37030691
It looks good to me, though some might argue any number you're not doing math with should be stored as a string/varchar.
0
 
LVL 4

Author Comment

by:asp_net2
ID: 37030767
Ok, now please overview this attached code. Same thing as I already attached except that now I need to add in my code that will send an email to the user once they submit the form. I'm not sure if it's good practice to include the email code into my ButtonClick Event Handler Code or keep it separate.




protected void btn_NewUser_Click(object sender, EventArgs e)
    {
        using (SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["HealthCourses"].ConnectionString))
        {
            SqlCommand cmd = new SqlCommand();
            cmd.CommandText = "HealthCourses_InsertUsers";
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Connection = conn;

            cmd.Parameters.AddWithValue("@c_id", SqlDbType.Int).Value = ddlClient.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@bldg_id", SqlDbType.Int).Value = ddlBldgLocation.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@s_id", SqlDbType.Int).Value = ddlState.SelectedItem.Value;
            cmd.Parameters.AddWithValue("@users_flname", SqlDbType.VarChar).Value = txtName.Text;
            cmd.Parameters.AddWithValue("@users_street_address", SqlDbType.VarChar).Value = txtHomeAddress.Text;
            cmd.Parameters.AddWithValue("@users_city", SqlDbType.VarChar).Value = txtCity.Text;
            cmd.Parameters.AddWithValue("@users_zip", SqlDbType.VarChar).Value = txtZipCode.Text;
            cmd.Parameters.AddWithValue("@users_phone", SqlDbType.VarChar).Value = txtHomePhone.Text;
            cmd.Parameters.AddWithValue("@users_work_ext", SqlDbType.VarChar).Value = txtWorkExtension.Text;
            cmd.Parameters.AddWithValue("@users_email", SqlDbType.VarChar).Value = txtEmailAddress.Text;
            cmd.Parameters.AddWithValue("@users_username", SqlDbType.VarChar).Value = txtUserName.Text;
            cmd.Parameters.AddWithValue("@users_password", SqlDbType.VarChar).Value = txtPassword.Text;

            try
            {
                conn.Open();

                cmd.ExecuteNonQuery();

                SmtpClient smtpClient = new SmtpClient();
                MailMessage message = new MailMessage();

                // Prepare email address
                MailAddress fromAddress = new MailAddress("no-reply@test.org", "Account Information");
                MailAddress toAddress = new MailAddress(HttpUtility.HtmlEncode(txtEmailAddress.Text));
                // Prepare the main message
                message.From = fromAddress;
                message.To.Add(toAddress);
                //message.CC.Add(ccAddress)

                message.Subject = "Login Information";
                message.IsBodyHtml = true;
                message.Body = "<html><head><title>" + "</title></head><body>" + "<p>" + "<span style=\"font-size: 14px; color: #780028; font-family: Arial\"><b>Your Email:&nbsp;</b><font face='arial' color='#666666'>" + HttpUtility.HtmlEncode(txtEmailAddress.Text) + "<br /><br />" + "<span style=\"font-size: 14px; color: #780028; font-family: Arial\"><b>Username:&nbsp;</b><font face='arial' color='#666666'>" + HttpUtility.HtmlEncode(txtUserName.Text) + "<br /><br />" + "<span style=\"font-size: 14px; color: #780028; font-family: Arial\"><br /><br />" + "Password are not retrievable, therefor they are unable to be emailed to you. Please make sure you remember your password" + "</body></html>";

                smtpClient.Host = "mail.test.org";
                smtpClient.Send(message);

                Response.Redirect("newuser_success.aspx");
            }

            catch (Exception ex)
            {
                lblInsertError.Text = ex.Message.ToString();
            }

            finally
            {
                conn.Close();
            }
        }
    }

Open in new window

0
 
LVL 35

Expert Comment

by:Paul MacDonald
ID: 37030901
I would generalize the e-mail code -  put it in its own class and have it accept parameters like recipient, sender, cc, bcc, subject, body, a boolean for HTML body type, and even mail server if you like.  That way you can easily resuse the code, in this application and in others.

That said, I'm not sure you'd want to HTMLEncode the "to" address.  Otherwise it looks okay to me.
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
LVL 4

Author Comment

by:asp_net2
ID: 37030950
Ok, thanks for looking all that over for me. Which brings me to this questions. If a user would click on the submit button more than once than an email is sent to the user as many times as he/she clicks on the submit button. At first I thought I was coding something wrong and that is why I wanted someone else to look at my code. But I can't figure out how to prevent multiple emails from being sent out. I was able to prevent multiple entrys into the DB in my stored procedure code but can't figure out how to prevent multiple emails from being sent if a user would click the button more than once.
0
 
LVL 35

Expert Comment

by:Paul MacDonald
ID: 37030976
You can disable or hide the button on postback, do your database work and - if your database work throws an exception - re-enable the button in the Catch.  If there's no exception thrown, the page refreshes without the button.  

You should probably put some text up that indicates the database was updated and an e-mail was sent too.
0
 
LVL 4

Author Comment

by:asp_net2
ID: 37031011
Hi paulmacd,

Can you assist me with the supplied code on how I can disable or hide button on postback unless database work throws and exception?
0
 
LVL 35

Accepted Solution

by:
Paul MacDonald earned 2000 total points
ID: 37031064
This is in VB, but you can convert it to C# easily:
 Protected Sub Page_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load
    ...
    btn_NewUser.Visible = False
    ...
  End Sub



Then in your existing code:
           ...
           try
            {
                conn.Open();

                cmd.ExecuteNonQuery();

                Response.Redirect("newuser_success.aspx");
            }

            catch (Exception ex)
            {
                btn_NewUser.Visible = True
                lblInsertError.Text = ex.Message.ToString();
            }

            finally
           ...


0
 
LVL 4

Author Comment

by:asp_net2
ID: 37031136
Not sure if I need to hide the button, maybe just disable it after user submits data and then if error raised then re-enable button.
0
 
LVL 35

Expert Comment

by:Paul MacDonald
ID: 37031182
btn_NewUser.Disabled = True/False then.
0

Featured Post

Keep up with what's happening at Experts Exchange!

Sign up to receive Decoded, a new monthly digest with product updates, feature release info, continuing education opportunities, and more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

It was really hard time for me to get the understanding of Delegates in C#. I went through many websites and articles but I found them very clumsy. After going through those sites, I noted down the points in a easy way so here I am sharing that unde…
International Data Corporation (IDC) prognosticates that before the current the year gets over disbursing on IT framework products to be sent in cloud environs will be $37.1B.
In a question here at Experts Exchange (https://www.experts-exchange.com/questions/29062564/Adobe-acrobat-reader-DC.html), a member asked how to create a signature in Adobe Acrobat Reader DC (the free Reader product, not the paid, full Acrobat produ…
With just a little bit of  SQL and VBA, many doors open to cool things like synchronize a list box to display data relevant to other information on a form.  If you have never written code or looked at an SQL statement before, no problem! ...  give i…
Suggested Courses

581 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question