SitePoint Sponsor

User Tag List

Results 1 to 8 of 8
  1. #1
    SitePoint Addict darkwater23's Avatar
    Join Date
    Nov 2005
    Location
    Omaha, NE
    Posts
    335
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Looking for proper technique

    I'm a .Net noob creating a new contact form. I updated this code from a 1.1 technique to a 2.0 technique.

    My first question is when the email is sent successfully, is Response.Redirect the "correct" way to move to the thank you page? (The code used to use views, but for SEO, I was asked to drop that technique.)

    My second question is related to the try/catch. If I comment out the Response.Redirect in the catch, it shows the Thank You page and I get the email. If I uncomment the Response.Redirect in the catch, it goes to the error page and I still get the email. What's the right way to do the following?

    Thanks!

    Code:
    private void processForm()
        {
            String emailTo = ConfigurationSettings.AppSettings["ContactToAddress"];
            String emailFrom = ConfigurationSettings.AppSettings["ContactFromAddress"];
            String emailSubject = "Contact Form Submission";
            String emailBody = "First Name: " + FirstName.Text + "\n" +
                               "Last Name: " + LastName.Text + "\n" +
                               "Company: " + Company.Text + "\n" +
                               "Phone: " + Phone.Text + "\n" +
                               "Email: " + Email.Text + "\n" +
                               "Department: " + ServiceInterest.SelectedItem.Value + "\n\n" +
                               "Comments:\n" + Comments.Text;
    
            SmtpClient client = new SmtpClient();
            MailMessage email = new MailMessage(emailFrom, emailTo, emailSubject, emailBody);
            email.Priority = MailPriority.High;
    
            try
            {
                client.Send(email);
                Response.Redirect("thank-you.aspx");
            }
            catch (Exception exc)
            {
                //literalErrorMessage.Text = exc.ToString();
                Response.Redirect("contact-error.aspx");
            }
        }

  2. #2
    SitePoint Wizard webcosmo's Avatar
    Join Date
    Oct 2007
    Location
    Boston, MA
    Posts
    1,480
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    why dont you redirect to the same url with a querystring to indicate success.

  3. #3
    SitePoint Addict darkwater23's Avatar
    Join Date
    Nov 2005
    Location
    Omaha, NE
    Posts
    335
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I could do that, but I'm concerned on why the try/catch isn't behaving as I expect it to. Do yo have any insight on that?

  4. #4
    SitePoint Evangelist
    Join Date
    Apr 2008
    Location
    Dublin, Ireland
    Posts
    461
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Response.redirect throws an exception because redirecting aborts the current thread. This is correct behavior. Hence it will always run the code in the catch so you should probably use a boolean variable and set it depending on success then after the try catch check this boolean and redirect accordingly.

  5. #5
    SitePoint Wizard
    Join Date
    Feb 2007
    Posts
    1,274
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Just move

    Response.Redirect("thank-you.aspx");

    out after the try-catch block.

  6. #6
    SitePoint Wizard
    Join Date
    Feb 2007
    Posts
    1,274
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Indeed, lose the try-catch all together. You do not need it. Any uncaught exceptions will invoke the standard error/exception behavior of ASP.NET. You configure in web.config which page to display. If the error page is an aspx page it has access to the exception details.

    Code:
    private void processForm()
        {
    
            ...
    
            client.Send(email);
            Response.Redirect("thank-you.aspx");
        }
    in web.config (example):

    Code:
    		<customErrors mode="On" defaultRedirect="error.aspx">
    			<error statusCode="403" redirect="NoAccess.htm"/>
    			<error statusCode="404" redirect="FileNotFound.htm"/>
    		</customErrors>

  7. #7
    SitePoint Addict darkwater23's Avatar
    Join Date
    Nov 2005
    Location
    Omaha, NE
    Posts
    335
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Smile

    That's a good idea! I was going to implement custom error pages anyway, so that will work well.

    Also, thanks for the insight on the Response.Redirect. That explains a lot.

    Thanks guys!

  8. #8
    ALT.NET - because we need it silver trophybronze trophy dhtmlgod's Avatar
    Join Date
    Jul 2001
    Location
    Scotland
    Posts
    4,836
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Just thought I would do a quick refactoring for to:

    - Improve Readability
    - Sort the Responsiblity of the methods by making smaller tightly focused methods
    - Maintainability, aided by the above, each method has only one reason to change

    Code Csharp:
    private void CreateAndSendEmail()
            {
                MailMessage mailMessage = CreateMailMessage();
                AddToAddress(mailMessage);
                AddFromAddrress(mailMessage);
                SetSubject(mailMessage, "Contact Form Submission");
                SetBody(mailMessage);
                SetPriority(mailMessage);
     
                SendMailMessage(mailMessage);
            }
     
            private MailMessage CreateMailMessage()
            {
                return new MailMessage();
            }
     
            private void AddToAddress(MailMessage message)
            {
                message.To.Add(new MailAddress(GetAppSettingsValue("ContactToAddress")));
            }
     
            private string GetAppSettingsValue(string key)
            {
                return ConfigurationSettings.AppSettings[key];
            }
     
            private void AddFromAddrress(MailMessage message)
            {
                message.From = new MailAddress(GetAppSettingsValue("ContactFromAddress"));
            }
     
            private void SetSubject(MailMessage message, string subject)
            {
                message.Subject = subject;
            }
     
            private void SetPriority(MailMessage message)
            {
                message.Priority = MailPriority.High;
            }
     
            private void SetBody(MailMessage message)
            {
                message.Body = "First Name: " + FirstName.Text + "\n" +
                               "Last Name: " + LastName.Text + "\n" +
                               "Company: " + Company.Text + "\n" +
                               "Phone: " + Phone.Text + "\n" +
                               "Email: " + Email.Text + "\n" +
                               "Department: " + ServiceInterest.SelectedItem.Value + "\n\n" +
                               "Comments:\n" + Comments.Text;
            }
     
            private void SendMailMessage(MailMessage mailMessage)
            {
                SmtpClient client = new SmtpClient();
                client.Send(mailMessage);
            }

    I personally would take this refactroing futher and abstract every as a it's own class and radically change the SetBody to accept a template. With ASP.NET MVC I use View's and the MVC ViewEngine


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •